Re: [Intel-gfx] ✗ Fi.CI.IGT: failure for tests/core_hotunplug: Fixes and enhancements (rev4)
On Fri, 2020-08-21 at 17:40 +, Patchwork wrote:
> Patch Details
> Series: tests/core_hotunplug: Fixes and enhancements (rev4)
> URL: https://patchwork.freedesktop.org/series/79671/
> State:failure
> Details: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4901/index.html
> CI Bug Log - changes from CI_DRM_8913_full -> IGTPW_4901_full
> Summary
> FAILURE
>
> Serious unknown changes coming with IGTPW_4901_full absolutely need to be
> verified manually.
>
> If you think the reported changes have nothing to do with the changes
> introduced in IGTPW_4901_full, please notify your bug team to allow them
> to document this new failure mode, which will reduce false positives in CI.
>
> External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4901/index.html
>
> Possible new issues
> Here are the unknown changes that may have been introduced in IGTPW_4901_full:
>
> IGT changes
> Possible regressions
> {igt@core_hotunplug@hotrebind-lateclose} (NEW):
>
> shard-snb: NOTRUN -> FAIL
>
> shard-glk: NOTRUN -> FAIL
>
> shard-kbl: NOTRUN -> FAIL
This is an existing but formerly not reported GPU hang driver issue
exhibited by the test, not a regression. The issue needs to be fixed
in the driver for the test to succeed. As one can see from CI reports,
the test succesfuly recovers from that condition and subsequent tests
are not affected.
>
> {igt@core_hotunplug@hotunbind-lateclose} (NEW):
>
> shard-hsw: NOTRUN -> INCOMPLETE +3 similar issues
This is a known driver issue, already reported by igt@device
_reset@unbind-reset-rebind. This test shows clearly that the issue has
nothing to do with device reset, only with driver unbind-rebind cycle.
The driver needs to be fixed for the kernel warning not be triggered
and the tests succeed.
> igt@gem_exec_flush@basic-uc-rw-default
>
> shard-snb: PASS -> FAIL
This issue seems hardly related to the igt@core_hotunplug test which is
the only one modified by the patch series. Even if popped up in the
same CI run as a GPU hang caused by igt@core_hotunplug@hotrebind-
lateclose, at least one subsequent test which depended on a healthy GPU
(igt@gem_exec_fence@syncobj-timeline-export) was run with success after
the igt@core_hotunplug@hotrebind-lateclos recovered successfully from
the GPU hang and before this failure occurred. Then I think we can't
blame core_hotunplug for this failure and we should treat the report of
this issue being a regression introduced by this patch series as a
false positive.
> igt@kms_flip@flip-vs-suspend-interruptible@c-edp1:
>
> shard-iclb: PASS -> INCOMPLETE
Even if this issue occurred in the same CI run as the core_hotunplug
was planned to be executed, it happened earlier and the run was
aborted. Then, there is no connection between both, so no regression
caused by this patch seriesa.
I think the *bind* subtests in their current shape are perfectly ready
for inclusion in CI runs.
Thanks,
Janusz
> New tests
> New tests have been introduced between CI_DRM_8913_full and IGTPW_4901_full:
>
> New IGT tests (4)
> igt@core_hotunplug@hotrebind-lateclose:
>
> Statuses : 4 fail(s) 1 incomplete(s) 1 pass(s)
> Exec time: [0.0, 16.20] s
> igt@core_hotunplug@hotunbind-lateclose:
>
> Statuses : 1 incomplete(s) 6 pass(s)
> Exec time: [0.0, 0.55] s
> igt@core_hotunplug@hotunbind-rebind:
>
> Statuses : 1 incomplete(s) 6 pass(s)
> Exec time: [0.0, 1.91] s
> igt@core_hotunplug@unbind-rebind:
>
> Statuses : 1 incomplete(s) 6 pass(s)
> Exec time: [0.0, 1.93] s
> Known issues
> Here are the changes found in IGTPW_4901_full that come from known issues:
>
> IGT changes
> Issues hit
> igt@i915_selftest@mock@contexts:
>
> shard-apl: PASS -> INCOMPLETE (i915#1635 / i915#2278)
> igt@kms_atomic_interruptible@universal-setplane-cursor:
>
> shard-hsw: PASS -> TIMEOUT (i915#1958) +1 similar issue
> igt@kms_big_fb@y-tiled-64bpp-rotate-0:
>
> shard-glk: PASS -> DMESG-FAIL (i915#118 / i915#95)
> igt@kms_flip@2x-plain-flip-fb-recreate@ac-hdmi-a1-hdmi-a2:
>
> shard-glk: PASS -> FAIL (i915#2122)
> igt@kms_flip@flip-vs-suspend@c-dp1:
>
> shard-kbl: PASS -> DMESG-WARN (i915#180) +8 similar issues
> igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
>
> shard-kbl: PASS -> DMESG-WARN (i915#1982) +2 similar issues
>
> shard-tglb: PASS -> DMESG-WARN (i915#1982) +1 similar issue
>
> igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-pwrite:
>
> shard-iclb: PASS -> FAIL (i915#49)
> igt@kms_hdr@bpc-switch-suspend:
>
> shard-glk: PASS -> DMESG-WARN (i915#1982)
> igt@kms_plane_cursor@pipe-a-overlay-size-256:
>
> shard-snb: PASS -> TIMEOUT (i915#1958) +1 similar issue
> igt@kms_psr2_su@frontbuffer:
>
> shard-iclb: PASS -> SKIP (fdo#109642 / fdo#111068)
> igt@kms_psr@psr2_primary_mmap_gtt:
>
> shard-iclb: PASS -> SKIP (fdo#109441)
> igt@prime_busy@before-wait@vecs0:
>
> shard-hsw: PASS -> FAIL (i915#2258) +1 similar issue
> Possible fixes
> igt@gem_exec_endless@dispatch@bcs0:
>
> shard-tglb: INCOMPLETE -> PASS
> igt@gem_exe
Re: [Intel-gfx] [PATCH 2/2] drm/i915/drrs: Disable DRRS when needed in fastsets
On 2020-08-20 at 22:53:53 +0530, José Roberto de Souza wrote:
> Changes in the configuration could cause PSR to be compatible and
> enabled so driver must also be able to disable DRRS when doing
> fastsets.
>
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/209
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/173
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/209
> Cc: Srinivas K
> Cc: Hariom Pandey
> Signed-off-by: José Roberto de Souza
overall patch looks goood to me,
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
> drivers/gpu/drm/i915/display/intel_dp.c | 84 +++-
> drivers/gpu/drm/i915/display/intel_dp.h | 2 +
> 3 files changed, 71 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index de5b216561d8..ff05a852417c 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -4012,7 +4012,7 @@ static void intel_ddi_update_pipe_dp(struct
> intel_atomic_state *state,
>
> intel_psr_update(intel_dp, crtc_state, conn_state);
> intel_dp_set_infoframes(encoder, true, crtc_state, conn_state);
> - intel_edp_drrs_enable(intel_dp, crtc_state);
> + intel_edp_drrs_update(intel_dp, crtc_state);
>
> intel_panel_update_backlight(state, encoder, crtc_state, conn_state);
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 3bf50b1ae983..de2c9851395d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2576,9 +2576,9 @@ intel_dp_compute_hdr_metadata_infoframe_sdp(struct
> intel_dp *intel_dp,
> }
>
> static void
> -intel_dp_drrs_compute_config(struct intel_dp *intel_dp,
> - struct intel_crtc_state *pipe_config,
> - int output_bpp, bool constant_n)
> +intel_dp_compute_drrs(struct intel_dp *intel_dp,
> + struct intel_crtc_state *pipe_config,
> + int output_bpp, bool constant_n)
intel_dp_drrs_compute_config looks a better name which u have introduced in
patch 1 of this series.
any reason to change the function name in second patch ?
> {
> struct intel_connector *intel_connector = intel_dp->attached_connector;
> struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> @@ -2586,8 +2586,8 @@ intel_dp_drrs_compute_config(struct intel_dp *intel_dp,
> /*
>* DRRS and PSR can't be enable together, so giving preference to PSR
>* as it allows more power-savings by complete shutting down display,
> - * so to guarantee this, intel_dp_drrs_compute_config() must be called
> - * after intel_psr_compute_config().
> + * so to guarantee this, intel_dp_compute_drrs() must be called after
> + * intel_psr_compute_config().
>*/
> if (pipe_config->has_psr)
> return;
> @@ -2688,8 +2688,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> intel_dp_set_clock(encoder, pipe_config);
>
> intel_psr_compute_config(intel_dp, pipe_config);
> - intel_dp_drrs_compute_config(intel_dp, pipe_config, output_bpp,
> - constant_n);
> + intel_dp_compute_drrs(intel_dp, pipe_config, output_bpp, constant_n);
> intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state);
> intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp, pipe_config,
> conn_state);
>
> @@ -7736,6 +7735,15 @@ static void intel_dp_set_drrs_state(struct
> drm_i915_private *dev_priv,
> refresh_rate);
> }
>
> +static void
> +intel_edp_drrs_enable_locked(struct intel_dp *intel_dp)
> +{
> + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> +
> + dev_priv->drrs.busy_frontbuffer_bits = 0;
> + dev_priv->drrs.dp = intel_dp;
> +}
> +
> /**
> * intel_edp_drrs_enable - init drrs struct if supported
> * @intel_dp: DP struct
> @@ -7752,19 +7760,34 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp,
> return;
>
> mutex_lock(&dev_priv->drrs.mutex);
> +
> if (dev_priv->drrs.dp) {
> - drm_dbg_kms(&dev_priv->drm, "DRRS already enabled\n");
> + drm_warn(&dev_priv->drm, "DRRS already enabled\n");
> goto unlock;
> }
>
> - dev_priv->drrs.busy_frontbuffer_bits = 0;
> -
> - dev_priv->drrs.dp = intel_dp;
> + intel_edp_drrs_enable_locked(intel_dp);
>
> unlock:
> mutex_unlock(&dev_priv->drrs.mutex);
> }
>
> +static void
> +intel_edp_drrs_disable_locked(struct intel_dp *intel_dp,
> + const struct intel_crtc_state *crtc_state)
> +{
> + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> +
> + if (dev_priv->drrs.refresh_rate_type == DRRS_LOW_RR) {
> + int refresh;
> +
> + refresh =
> drm_mode_vrefresh(inte
Re: [Intel-gfx] ✗ Fi.CI.IGT: failure for tests/core_hotunplug: Fixes and enhancements (rev4)
On Mon, Aug 24, 2020 at 10:42:10AM +0200, Janusz Krzysztofik wrote:
> On Fri, 2020-08-21 at 17:40 +, Patchwork wrote:
> > Patch Details
> > Series: tests/core_hotunplug: Fixes and enhancements (rev4)
> > URL:https://patchwork.freedesktop.org/series/79671/
> > State: failure
> > Details:https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4901/index.html
> > CI Bug Log - changes from CI_DRM_8913_full -> IGTPW_4901_full
> > Summary
> > FAILURE
> >
> > Serious unknown changes coming with IGTPW_4901_full absolutely need to be
> > verified manually.
> >
> > If you think the reported changes have nothing to do with the changes
> > introduced in IGTPW_4901_full, please notify your bug team to allow them
> > to document this new failure mode, which will reduce false positives in CI.
> >
> > External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4901/index.html
> >
> > Possible new issues
> > Here are the unknown changes that may have been introduced in
> > IGTPW_4901_full:
> >
> > IGT changes
> > Possible regressions
> > {igt@core_hotunplug@hotrebind-lateclose} (NEW):
> >
> > shard-snb: NOTRUN -> FAIL
> >
> > shard-glk: NOTRUN -> FAIL
> >
> > shard-kbl: NOTRUN -> FAIL
>
> This is an existing but formerly not reported GPU hang driver issue
> exhibited by the test, not a regression. The issue needs to be fixed
> in the driver for the test to succeed. As one can see from CI reports,
> the test succesfuly recovers from that condition and subsequent tests
> are not affected.
>
> >
> > {igt@core_hotunplug@hotunbind-lateclose} (NEW):
> >
> > shard-hsw: NOTRUN -> INCOMPLETE +3 similar issues
>
> This is a known driver issue, already reported by igt@device
> _reset@unbind-reset-rebind. This test shows clearly that the issue has
> nothing to do with device reset, only with driver unbind-rebind cycle.
> The driver needs to be fixed for the kernel warning not be triggered
> and the tests succeed.
Is there an ETA for the driver fix?
>
> I think the *bind* subtests in their current shape are perfectly ready
> for inclusion in CI runs.
Agreed otherwise, except for the known incomplete. Introducing new
incompletes without a fix in sight is real bad.
--
Petri Latvala
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ Fi.CI.IGT: failure for tests/core_hotunplug: Fixes and enhancements (rev4)
Hi Petri,
On Mon, 2020-08-24 at 12:26 +0300, Petri Latvala wrote:
> On Mon, Aug 24, 2020 at 10:42:10AM +0200, Janusz Krzysztofik wrote:
> > On Fri, 2020-08-21 at 17:40 +, Patchwork wrote:
> > > Patch Details
> > > Series: tests/core_hotunplug: Fixes and enhancements (rev4)
> > > URL: https://patchwork.freedesktop.org/series/79671/
> > > State:failure
> > > Details: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4901/index.html
> > > CI Bug Log - changes from CI_DRM_8913_full -> IGTPW_4901_full
> > > Summary
> > > FAILURE
> > >
> > > Serious unknown changes coming with IGTPW_4901_full absolutely need to be
> > > verified manually.
> > >
> > > If you think the reported changes have nothing to do with the changes
> > > introduced in IGTPW_4901_full, please notify your bug team to allow them
> > > to document this new failure mode, which will reduce false positives in
> > > CI.
> > >
> > > External URL:
> > > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4901/index.html
> > >
> > > Possible new issues
> > > Here are the unknown changes that may have been introduced in
> > > IGTPW_4901_full:
> > >
> > > IGT changes
> > > Possible regressions
> > > {igt@core_hotunplug@hotrebind-lateclose} (NEW):
> > >
> > > shard-snb: NOTRUN -> FAIL
> > >
> > > shard-glk: NOTRUN -> FAIL
> > >
> > > shard-kbl: NOTRUN -> FAIL
> >
> > This is an existing but formerly not reported GPU hang driver issue
> > exhibited by the test, not a regression. The issue needs to be fixed
> > in the driver for the test to succeed. As one can see from CI reports,
> > the test succesfuly recovers from that condition and subsequent tests
> > are not affected.
> >
> > > {igt@core_hotunplug@hotunbind-lateclose} (NEW):
> > >
> > > shard-hsw: NOTRUN -> INCOMPLETE +3 similar issues
> >
> > This is a known driver issue, already reported by igt@device
> > _reset@unbind-reset-rebind. This test shows clearly that the issue has
> > nothing to do with device reset, only with driver unbind-rebind cycle.
> > The driver needs to be fixed for the kernel warning not be triggered
> > and the tests succeed.
>
> Is there an ETA for the driver fix?
Not that I know of. I'll have a look at it.
Thanks,
Janusz
>
> > I think the *bind* subtests in their current shape are perfectly ready
> > for inclusion in CI runs.
>
> Agreed otherwise, except for the known incomplete. Introducing new
> incompletes without a fix in sight is real bad.
>
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/5] Fixing Possible Null Pointer Dereference
There is a possble Null Pointer dereference in intel_atomic.c and this patch fixes the same by introducting a check to old_state, new_state old_conn_state and new_conn_state variables. Signed-off-by: Nischal Varide --- drivers/gpu/drm/i915/display/intel_atomic.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c index 630f49b7aa01..ab58f061c8a7 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic.c +++ b/drivers/gpu/drm/i915/display/intel_atomic.c @@ -132,6 +132,9 @@ int intel_digital_connector_atomic_check(struct drm_connector *conn, to_intel_digital_connector_state(old_state); struct drm_crtc_state *crtc_state; + if (!(new_state && new_conn_state && old_state && old_conn_state)) + return 0; + intel_hdcp_atomic_check(conn, old_state, new_state); intel_psr_atomic_check(conn, old_state, new_state); @@ -192,6 +195,8 @@ intel_connector_needs_modeset(struct intel_atomic_state *state, old_conn_state = drm_atomic_get_old_connector_state(&state->base, connector); new_conn_state = drm_atomic_get_new_connector_state(&state->base, connector); + if (!(old_conn_state && new_conn_state)) + return 0; return old_conn_state->crtc != new_conn_state->crtc || (new_conn_state->crtc && -- 2.26.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/5] Fixing Possible Null Pointer Dereference.
A possible Null Pointer dereference of new_state,old_state, new_crtc_state
pointers from intel_tv.c is handled in this patch.
Signed-off-by: Nischal Varide
---
drivers/gpu/drm/i915/display/intel_tv.c | 17 ++---
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_tv.c
b/drivers/gpu/drm/i915/display/intel_tv.c
index 777032d9697b..5600d146ae81 100644
--- a/drivers/gpu/drm/i915/display/intel_tv.c
+++ b/drivers/gpu/drm/i915/display/intel_tv.c
@@ -1836,17 +1836,20 @@ static int intel_tv_atomic_check(struct drm_connector
*connector,
struct drm_connector_state *old_state;
new_state = drm_atomic_get_new_connector_state(state, connector);
- if (!new_state->crtc)
- return 0;
-
old_state = drm_atomic_get_old_connector_state(state, connector);
new_crtc_state = drm_atomic_get_new_crtc_state(state, new_state->crtc);
+ if (!(old_state && new_state && new_crtc_state))
+ return 0;
+
+ if (!new_state->crtc)
+ return 0;
+
if (old_state->tv.mode != new_state->tv.mode ||
- old_state->tv.margins.left != new_state->tv.margins.left ||
- old_state->tv.margins.right != new_state->tv.margins.right ||
- old_state->tv.margins.top != new_state->tv.margins.top ||
- old_state->tv.margins.bottom != new_state->tv.margins.bottom) {
+ old_state->tv.margins.left != new_state->tv.margins.left ||
+ old_state->tv.margins.right != new_state->tv.margins.right ||
+ old_state->tv.margins.top != new_state->tv.margins.top ||
+ old_state->tv.margins.bottom != new_state->tv.margins.bottom) {
/* Force a modeset. */
new_crtc_state->connectors_changed = true;
--
2.26.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/5] Fixing Possible Null Pointer Dereference.
A Possible Null Pointer Dereference of a Pointer obj is handled in
intel_displa.c .This patch introduced a check on pointer obj before
dereferencing
Signed-off-by: Nischal Varide
---
drivers/gpu/drm/i915/display/intel_display.c | 22 +++-
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 7d50b7177d40..30d5ab3f098d 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -2265,9 +2265,9 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
pinctl = 0;
if (HAS_GMCH(dev_priv))
pinctl |= PIN_MAPPABLE;
-
- vma = i915_gem_object_pin_to_display_plane(obj,
- alignment, view, pinctl);
+ if (obj)
+ vma = i915_gem_object_pin_to_display_plane(obj,
+ alignment, view, pinctl);
if (IS_ERR(vma))
goto err;
@@ -11309,10 +11309,13 @@ static u32 intel_cursor_base(const struct
intel_plane_state *plane_state)
const struct drm_i915_gem_object *obj = intel_fb_obj(fb);
u32 base;
- if (INTEL_INFO(dev_priv)->display.cursor_needs_physical)
- base = sg_dma_address(obj->mm.pages->sgl);
- else
- base = intel_plane_ggtt_offset(plane_state);
+ if (obj) {
+
+ if (INTEL_INFO(dev_priv)->display.cursor_needs_physical)
+ base = sg_dma_address(obj->mm.pages->sgl);
+ else
+ base = intel_plane_ggtt_offset(plane_state);
+ }
return base + plane_state->color_plane[0].offset;
}
@@ -17166,10 +17169,9 @@ static int intel_user_framebuffer_dirty(struct
drm_framebuffer *fb,
unsigned num_clips)
{
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
-
- i915_gem_object_flush_if_display(obj);
+ if (obj)
+ i915_gem_object_flush_if_display(obj);
intel_frontbuffer_flush(to_intel_frontbuffer(fb), ORIGIN_DIRTYFB);
-
return 0;
}
--
2.26.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/5] Fixing a Possible Null Pointer Dereference.
This Patch addresses a Possible Null Pointer Dereference of variables
new_stae, new_conn_state and old_state, old_conn_state in intel_sdvo.c
Signed-off-by: Nischal Varide
---
drivers/gpu/drm/i915/display/intel_sdvo.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
b/drivers/gpu/drm/i915/display/intel_sdvo.c
index 2da4388e1540..186a2d695bb6 100644
--- a/drivers/gpu/drm/i915/display/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
@@ -2505,6 +2505,9 @@ static int intel_sdvo_atomic_check(struct drm_connector
*conn,
struct intel_sdvo_connector_state *new_state =
to_intel_sdvo_connector_state(new_conn_state);
+ if (!(new_conn_state && old_conn_state && old_state && new_state))
+ return 0;
+
if (new_conn_state->crtc &&
(memcmp(&old_state->tv, &new_state->tv, sizeof(old_state->tv)) ||
memcmp(&old_conn_state->tv, &new_conn_state->tv,
sizeof(old_conn_state->tv {
--
2.26.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/5] Fixing a Possible Null Pointer Dereference.
Fixing a Possible Null Pointer Dereference in intel_crt.c. Introduced a Null Check for dev_priv pointer before dereferencing. Signed-off-by: Nischal Varide --- drivers/gpu/drm/i915/display/intel_crt.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c index 5b4510ce5693..b5777fdc8cac 100644 --- a/drivers/gpu/drm/i915/display/intel_crt.c +++ b/drivers/gpu/drm/i915/display/intel_crt.c @@ -504,6 +504,9 @@ static bool valleyview_crt_detect_hotplug(struct drm_connector *connector) bool ret; u32 save_adpa; + if (!(dev_priv)) + return false; + /* * Doing a force trigger causes a hpd interrupt to get sent, which can * get us stuck in a loop if we're polling: -- 2.26.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/5] Fixing Possible Null Pointer Dereference
== Series Details ==
Series: series starting with [1/5] Fixing Possible Null Pointer Dereference
URL : https://patchwork.freedesktop.org/series/80939/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
e2dc6f705c6e Fixing Possible Null Pointer Dereference
3b7087647897 Fixing Possible Null Pointer Dereference.
-:25: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#25: FILE: drivers/gpu/drm/i915/display/intel_display.c:2270:
+ vma = i915_gem_object_pin_to_display_plane(obj,
+ alignment, view, pinctl);
-:38: CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
#38: FILE: drivers/gpu/drm/i915/display/intel_display.c:11313:
+ if (obj) {
+
total: 0 errors, 0 warnings, 2 checks, 41 lines checked
392417891b5b Fixing a Possible Null Pointer Dereference.
932de90c59ae Fixing Possible Null Pointer Dereference.
fe514036c049 Fixing a Possible Null Pointer Dereference.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Break up error capture compression loops with cond_resched()
Chris Wilson writes: > As the error capture will compress user buffers as directed to by the > user, it can take an arbitrary amount of time and space. Break up the > compression loops with a call to cond_resched(), that will allow other > processes to schedule (avoiding the soft lockups) and also serve as a > warning should we try to make this loop atomic in the future. > > Signed-off-by: Chris Wilson > Cc: Mika Kuoppala > Cc: sta...@vger.kernel.org Reviewed-by: Mika Kuoppala > --- > drivers/gpu/drm/i915/i915_gpu_error.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c > b/drivers/gpu/drm/i915/i915_gpu_error.c > index 6a3a2ce0b394..6551ff04d5a6 100644 > --- a/drivers/gpu/drm/i915/i915_gpu_error.c > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c > @@ -311,6 +311,8 @@ static int compress_page(struct i915_vma_compress *c, > > if (zlib_deflate(zstream, Z_NO_FLUSH) != Z_OK) > return -EIO; > + > + cond_resched(); > } while (zstream->avail_in); > > /* Fallback to uncompressed if we increase size? */ > @@ -397,6 +399,7 @@ static int compress_page(struct i915_vma_compress *c, > if (!(wc && i915_memcpy_from_wc(ptr, src, PAGE_SIZE))) > memcpy(ptr, src, PAGE_SIZE); > dst->pages[dst->page_count++] = ptr; > + cond_resched(); > > return 0; > } > -- > 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Compute has_drrs after compute has_psr
On 2020-08-20 at 10:23:52 -0700, José Roberto de Souza wrote:
> DRRS and PSR can't be enable together, so giving preference to PSR
> as it allows more power-savings by complete shutting down display,
> so to guarantee this, it should compute DRRS state after compute PSR.
>
> Cc: Srinivas K
> Cc: Hariom Pandey
> Signed-off-by: José Roberto de Souza
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 52 +++--
> 1 file changed, 31 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 79c27f91f42c..3bf50b1ae983 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2575,6 +2575,34 @@ intel_dp_compute_hdr_metadata_infoframe_sdp(struct
> intel_dp *intel_dp,
> intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_GAMUT_METADATA);
> }
>
> +static void
> +intel_dp_drrs_compute_config(struct intel_dp *intel_dp,
> + struct intel_crtc_state *pipe_config,
> + int output_bpp, bool constant_n)
> +{
> + struct intel_connector *intel_connector = intel_dp->attached_connector;
> + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> +
> + /*
> + * DRRS and PSR can't be enable together, so giving preference to PSR
> + * as it allows more power-savings by complete shutting down display,
> + * so to guarantee this, intel_dp_drrs_compute_config() must be called
> + * after intel_psr_compute_config().
> + */
> + if (pipe_config->has_psr)
> + return;
In cases when psr is supported but is not enabled due to any psr error, drrs
can be useful.
AFAIU has_psr is false in cases psr is disabled due any PSR error, this should
be fine for such case
> +
> + if (!intel_connector->panel.downclock_mode ||
> + dev_priv->drrs.type != SEAMLESS_DRRS_SUPPORT)
> + return;
> +
> + pipe_config->has_drrs = true;
> + intel_link_compute_m_n(output_bpp, pipe_config->lane_count,
> +intel_connector->panel.downclock_mode->clock,
> +pipe_config->port_clock, &pipe_config->dp_m2_n2,
> +constant_n, pipe_config->fec_enable);
> +}
> +
> int
> intel_dp_compute_config(struct intel_encoder *encoder,
> struct intel_crtc_state *pipe_config,
> @@ -2605,7 +2633,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> if (ret)
> return ret;
>
> - pipe_config->has_drrs = false;
May be u can retain it for better readability.
> if (!intel_dp_port_has_audio(dev_priv, port))
> pipe_config->has_audio = false;
> else if (intel_conn_state->force_audio == HDMI_AUDIO_AUTO)
> @@ -2657,21 +2684,12 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> &pipe_config->dp_m_n,
> constant_n, pipe_config->fec_enable);
>
> - if (intel_connector->panel.downclock_mode != NULL &&
> - dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
> - pipe_config->has_drrs = true;
> - intel_link_compute_m_n(output_bpp,
> -pipe_config->lane_count,
> -
> intel_connector->panel.downclock_mode->clock,
> -pipe_config->port_clock,
> -&pipe_config->dp_m2_n2,
> -constant_n,
> pipe_config->fec_enable);
> - }
> -
> if (!HAS_DDI(dev_priv))
> intel_dp_set_clock(encoder, pipe_config);
>
> intel_psr_compute_config(intel_dp, pipe_config);
> + intel_dp_drrs_compute_config(intel_dp, pipe_config, output_bpp,
> + constant_n);
> intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state);
> intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp, pipe_config,
> conn_state);
>
> @@ -7730,16 +7748,8 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp,
> {
> struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>
> - if (!crtc_state->has_drrs) {
> - drm_dbg_kms(&dev_priv->drm, "Panel doesn't support DRRS\n");
IMHO this print is useful.
with all of above,
Reviewed-by: Anshuman Gupta
> + if (!crtc_state->has_drrs)
> return;
> - }
> -
> - if (dev_priv->psr.enabled) {
> - drm_dbg_kms(&dev_priv->drm,
> - "PSR enabled. Not enabling DRRS.\n");
> - return;
> - }
>
> mutex_lock(&dev_priv->drrs.mutex);
> if (dev_priv->drrs.dp) {
> --
> 2.28.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/5] Fixing Possible Null Pointer Dereference
== Series Details == Series: series starting with [1/5] Fixing Possible Null Pointer Dereference URL : https://patchwork.freedesktop.org/series/80939/ State : success == Summary == CI Bug Log - changes from CI_DRM_8918 -> Patchwork_18392 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18392/index.html Known issues Here are the changes found in Patchwork_18392 that come from known issues: ### IGT changes ### Issues hit * igt@i915_pm_rpm@module-reload: - fi-apl-guc: [PASS][1] -> [DMESG-WARN][2] ([i915#1635] / [i915#1982]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/fi-apl-guc/igt@i915_pm_...@module-reload.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18392/fi-apl-guc/igt@i915_pm_...@module-reload.html Possible fixes * igt@i915_pm_rpm@basic-pci-d3-state: - fi-bsw-n3050: [DMESG-WARN][3] ([i915#1982]) -> [PASS][4] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/fi-bsw-n3050/igt@i915_pm_...@basic-pci-d3-state.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18392/fi-bsw-n3050/igt@i915_pm_...@basic-pci-d3-state.html - fi-bsw-kefka: [DMESG-WARN][5] ([i915#1982]) -> [PASS][6] [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/fi-bsw-kefka/igt@i915_pm_...@basic-pci-d3-state.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18392/fi-bsw-kefka/igt@i915_pm_...@basic-pci-d3-state.html * igt@kms_flip@basic-flip-vs-wf_vblank@c-hdmi-a2: - fi-skl-guc: [DMESG-WARN][7] ([i915#2203]) -> [PASS][8] [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/fi-skl-guc/igt@kms_flip@basic-flip-vs-wf_vbl...@c-hdmi-a2.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18392/fi-skl-guc/igt@kms_flip@basic-flip-vs-wf_vbl...@c-hdmi-a2.html Warnings * igt@kms_force_connector_basic@force-connector-state: - fi-kbl-x1275: [DMESG-WARN][9] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][10] ([i915#62] / [i915#92]) +3 similar issues [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/fi-kbl-x1275/igt@kms_force_connector_ba...@force-connector-state.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18392/fi-kbl-x1275/igt@kms_force_connector_ba...@force-connector-state.html * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a: - fi-kbl-x1275: [DMESG-WARN][11] ([i915#62] / [i915#92]) -> [DMESG-WARN][12] ([i915#62] / [i915#92] / [i915#95]) +3 similar issues [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/fi-kbl-x1275/igt@kms_pipe_crc_ba...@suspend-read-crc-pipe-a.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18392/fi-kbl-x1275/igt@kms_pipe_crc_ba...@suspend-read-crc-pipe-a.html [i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635 [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982 [i915#2203]: https://gitlab.freedesktop.org/drm/intel/issues/2203 [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62 [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92 [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95 Participating hosts (37 -> 34) -- Missing(3): fi-byt-clapper fi-byt-squawks fi-bsw-cyan Build changes - * Linux: CI_DRM_8918 -> Patchwork_18392 CI-20190529: 20190529 CI_DRM_8918: 7296072da95d2d2550ef2d4836bc72663ee711cc @ git://anongit.freedesktop.org/gfx-ci/linux IGT_5770: f1d0c240ea2e631dfb9f493f37f8fb61cb2b1cf2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_18392: fe514036c04927db4d21ec423cc1d3ff9747700e @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == fe514036c049 Fixing a Possible Null Pointer Dereference. 932de90c59ae Fixing Possible Null Pointer Dereference. 392417891b5b Fixing a Possible Null Pointer Dereference. 3b7087647897 Fixing Possible Null Pointer Dereference. e2dc6f705c6e Fixing Possible Null Pointer Dereference == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18392/index.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 00/16] acpi/pwm/i915: Convert pwm-crc and i915 driver's PWM code to use the atomic PWM API
Hi All, Here is v6 of my patch series converting the i915 driver's code for controlling the panel's backlight with an external PWM controller to use the atomic PWM API. See below for the changelog. This version of the series has been rebased on 5.9-rc1 and has a Reviewed-by or Acked-by for all patches. The main purpose of sending this new version out is to allow the intel-gfx CI to play with it. As discussed before, because of interdependencies of the patches I plan to push the entire series to drm-intel-next-queued once it has passed CI. Thierry, I believe from our previous discussion that you are ok with pushing the pwm-crc and pwm-lpss patches through the drm-intel tree, but you have not given your Acked-by for this. If you are not ok with me pushing these out this way please let me now ASAP. If you are ok with this an Acked-by would be appreciated. This series has been tested (and re-tested after adding various bug-fixes) extensively. It has been tested on the following devices: -Asus T100TA BYT + CRC-PMIC PWM -Toshiba WT8-A BYT + CRC-PMIC PWM -Thundersoft TS178 BYT + CRC-PMIC PWM, inverse PWM -Asus T100HA CHT + CRC-PMIC PWM -Terra Pad 1061 BYT + LPSS PWM -Trekstor Twin 10.1 BYT + LPSS PWM -Asus T101HA CHT + CRC-PMIC PWM -GPD Pocket CHT + CRC-PMIC PWM Regards, Hans Changelog: Changes in v6: - Rebase on v5.9-rc1 - Adjust pwm-crc patches for pwm_state.period and .duty_cycle now being u64 Changes in v5: - Dropped the "pwm: lpss: Correct get_state result for base_unit == 0" patch. The base_unit == 0 condition should never happen and sofar it is unclear what the proper behavior / correct values to store in the pwm_state should be when this does happen. Since this patch was added as an extra pwm-lpss fix in v4 of this patch-set and otherwise is orthogonal to the of this patch-set just drop it (again). - "[PATCH 04/16] pwm: lpss: Add range limit check for the base_unit register value" - Use clamp_val(... instead of clam_t(unsigned long long, ... - "[PATCH 05/16] pwm: lpss: Add pwm_lpss_prepare_enable() helper" - This is a new patch in v5 of this patchset - [PATCH 06/16] pwm: lpss: Use pwm_lpss_apply() when restoring state on resume - Use the new pwm_lpss_prepare_enable() helper Changes in v4: - "[PATCH v4 06/16] pwm: lpss: Correct get_state result for base_unit == 0" - This is a new patch in v4 of this patchset - "[PATCH v4 12/16] pwm: crc: Implement get_state() method" - Use DIV_ROUND_UP when calculating the period and duty_cycle values - "[PATCH v4 16/16] drm/i915: panel: Use atomic PWM API for devs with an external PWM controller" - Add a note to the commit message about the changes in pwm_disable_backlight() - Use the pwm_set/get_relative_duty_cycle() helpers Changes in v3: - "[PATCH v3 04/15] pwm: lpss: Add range limit check for the base_unit register value" - Use base_unit_range - 1 as maximum value for the clamp() - "[PATCH v3 05/15] pwm: lpss: Use pwm_lpss_apply() when restoring state on resume" - This replaces the "pwm: lpss: Set SW_UPDATE bit when enabling the PWM" patch from previous versions of this patch-set, which really was a hack working around the resume issue which this patch fixes properly. - PATCH v3 6 - 11 pwm-crc changes: - Various small changes resulting from the reviews by Andy and Uwe, including some refactoring of the patches to reduce the amount of churn in the patch-set Changes in v2: - Fix coverletter subject - Drop accidentally included debugging patch - "[PATCH v3 02/15] ACPI / LPSS: Save Cherry Trail PWM ctx registers only once ( - Move #define LPSS_SAVE_CTX_ONCE define to group it with LPSS_SAVE_CTX ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 05/16] pwm: lpss: Add pwm_lpss_prepare_enable() helper
In the not-enabled -> enabled path pwm_lpss_apply() needs to get a
runtime-pm reference; and then on any errors it needs to release it
again.
This leads to somewhat hard to read code. This commit introduces a new
pwm_lpss_prepare_enable() helper and moves all the steps necessary for
the not-enabled -> enabled transition there, so that we can error check
the entire transition in a single place and only have one pm_runtime_put()
on failure call site.
While working on this I noticed that the enabled -> enabled (update
settings) path was quite similar, so I've added an enable parameter to
the new pwm_lpss_prepare_enable() helper, which allows using it in that
path too.
Suggested-by: Andy Shevchenko
Reviewed-by: Andy Shevchenko
Signed-off-by: Hans de Goede
---
drivers/pwm/pwm-lpss.c | 45 --
1 file changed, 26 insertions(+), 19 deletions(-)
diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c
index da9bc3d10104..8a136ba2a583 100644
--- a/drivers/pwm/pwm-lpss.c
+++ b/drivers/pwm/pwm-lpss.c
@@ -122,41 +122,48 @@ static inline void pwm_lpss_cond_enable(struct pwm_device
*pwm, bool cond)
pwm_lpss_write(pwm, pwm_lpss_read(pwm) | PWM_ENABLE);
}
+static int pwm_lpss_prepare_enable(struct pwm_lpss_chip *lpwm,
+ struct pwm_device *pwm,
+ const struct pwm_state *state,
+ bool enable)
+{
+ int ret;
+
+ ret = pwm_lpss_is_updating(pwm);
+ if (ret)
+ return ret;
+
+ pwm_lpss_prepare(lpwm, pwm, state->duty_cycle, state->period);
+ pwm_lpss_cond_enable(pwm, enable && lpwm->info->bypass == false);
+ ret = pwm_lpss_wait_for_update(pwm);
+ if (ret)
+ return ret;
+
+ pwm_lpss_cond_enable(pwm, enable && lpwm->info->bypass == true);
+ return 0;
+}
+
static int pwm_lpss_apply(struct pwm_chip *chip, struct pwm_device *pwm,
const struct pwm_state *state)
{
struct pwm_lpss_chip *lpwm = to_lpwm(chip);
- int ret;
+ int ret = 0;
if (state->enabled) {
if (!pwm_is_enabled(pwm)) {
pm_runtime_get_sync(chip->dev);
- ret = pwm_lpss_is_updating(pwm);
- if (ret) {
- pm_runtime_put(chip->dev);
- return ret;
- }
- pwm_lpss_prepare(lpwm, pwm, state->duty_cycle,
state->period);
- pwm_lpss_cond_enable(pwm, lpwm->info->bypass == false);
- ret = pwm_lpss_wait_for_update(pwm);
- if (ret) {
+ ret = pwm_lpss_prepare_enable(lpwm, pwm, state, true);
+ if (ret)
pm_runtime_put(chip->dev);
- return ret;
- }
- pwm_lpss_cond_enable(pwm, lpwm->info->bypass == true);
} else {
- ret = pwm_lpss_is_updating(pwm);
- if (ret)
- return ret;
- pwm_lpss_prepare(lpwm, pwm, state->duty_cycle,
state->period);
- return pwm_lpss_wait_for_update(pwm);
+ ret = pwm_lpss_prepare_enable(lpwm, pwm, state, false);
}
} else if (pwm_is_enabled(pwm)) {
pwm_lpss_write(pwm, pwm_lpss_read(pwm) & ~PWM_ENABLE);
pm_runtime_put(chip->dev);
}
- return 0;
+ return ret;
}
static void pwm_lpss_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
--
2.28.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 04/16] pwm: lpss: Add range limit check for the base_unit register value
When the user requests a high enough period ns value, then the
calculations in pwm_lpss_prepare() might result in a base_unit value of 0.
But according to the data-sheet the way the PWM controller works is that
each input clock-cycle the base_unit gets added to a N bit counter and
that counter overflowing determines the PWM output frequency. Adding 0
to the counter is a no-op. The data-sheet even explicitly states that
writing 0 to the base_unit bits will result in the PWM outputting a
continuous 0 signal.
When the user requestes a low enough period ns value, then the
calculations in pwm_lpss_prepare() might result in a base_unit value
which is bigger then base_unit_range - 1. Currently the codes for this
deals with this by applying a mask:
base_unit &= (base_unit_range - 1);
But this means that we let the value overflow the range, we throw away the
higher bits and store whatever value is left in the lower bits into the
register leading to a random output frequency, rather then clamping the
output frequency to the highest frequency which the hardware can do.
This commit fixes both issues by clamping the base_unit value to be
between 1 and (base_unit_range - 1).
Fixes: 684309e5043e ("pwm: lpss: Avoid potential overflow of base_unit")
Reviewed-by: Andy Shevchenko
Signed-off-by: Hans de Goede
---
Changes in v5:
- Use clamp_val(... instead of clam_t(unsigned long long, ...
Changes in v3:
- Change upper limit of clamp to (base_unit_range - 1)
- Add Fixes tag
---
drivers/pwm/pwm-lpss.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c
index 43b1fc634af1..da9bc3d10104 100644
--- a/drivers/pwm/pwm-lpss.c
+++ b/drivers/pwm/pwm-lpss.c
@@ -97,6 +97,8 @@ static void pwm_lpss_prepare(struct pwm_lpss_chip *lpwm,
struct pwm_device *pwm,
freq *= base_unit_range;
base_unit = DIV_ROUND_CLOSEST_ULL(freq, c);
+ /* base_unit must not be 0 and we also want to avoid overflowing it */
+ base_unit = clamp_val(base_unit, 1, base_unit_range - 1);
on_time_div = 255ULL * duty_ns;
do_div(on_time_div, period_ns);
@@ -105,7 +107,6 @@ static void pwm_lpss_prepare(struct pwm_lpss_chip *lpwm,
struct pwm_device *pwm,
orig_ctrl = ctrl = pwm_lpss_read(pwm);
ctrl &= ~PWM_ON_TIME_DIV_MASK;
ctrl &= ~((base_unit_range - 1) << PWM_BASE_UNIT_SHIFT);
- base_unit &= (base_unit_range - 1);
ctrl |= (u32) base_unit << PWM_BASE_UNIT_SHIFT;
ctrl |= on_time_div;
--
2.28.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 09/16] pwm: crc: Fix period changes not having any effect
The pwm-crc code is using 2 different enable bits:
1. bit 7 of the PWM0_CLK_DIV (PWM_OUTPUT_ENABLE)
2. bit 0 of the BACKLIGHT_EN register
The BACKLIGHT_EN register at address 0x51 really controls a separate
output-only GPIO which is earmarked to be used as output connected to the
backlight-enable pin for LCD panels, this GPO is part of the PMIC's
"Display Panel Control Block." . This pin should probably be moved over
to a GPIO provider driver (and consumers modified accordingly), but that
is something for an(other) patch.
Enabling / disabling the actual PWM output is controlled by the
PWM_OUTPUT_ENABLE bit of the PWM0_CLK_DIV register.
As the comment in the old code already indicates we must disable the PWM
before we can change the clock divider. But the crc_pwm_disable() and
crc_pwm_enable() calls the old code make for this only change the
BACKLIGHT_EN register; and the value of that register does not matter for
changing the period / the divider. What does matter is that the
PWM_OUTPUT_ENABLE bit must be cleared before a new value can be written.
This commit modifies crc_pwm_config() to clear PWM_OUTPUT_ENABLE instead
when changing the period, so that period changes actually work.
Note this fix will cause a significant behavior change on some devices
using the CRC PWM output to drive their backlight. Before the PWM would
always run with the output frequency configured by the BIOS at boot, now
the period time specified by the i915 driver will actually be honored.
Reviewed-by: Andy Shevchenko
Signed-off-by: Hans de Goede
---
drivers/pwm/pwm-crc.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/pwm/pwm-crc.c b/drivers/pwm/pwm-crc.c
index 44ec7d5b63e1..81232da0c767 100644
--- a/drivers/pwm/pwm-crc.c
+++ b/drivers/pwm/pwm-crc.c
@@ -82,14 +82,11 @@ static int crc_pwm_config(struct pwm_chip *c, struct
pwm_device *pwm,
if (pwm_get_period(pwm) != period_ns) {
int clk_div = crc_pwm_calc_clk_div(period_ns);
- /* changing the clk divisor, need to disable fisrt */
- crc_pwm_disable(c, pwm);
+ /* changing the clk divisor, clear PWM_OUTPUT_ENABLE first */
+ regmap_write(crc_pwm->regmap, PWM0_CLK_DIV, 0);
regmap_write(crc_pwm->regmap, PWM0_CLK_DIV,
clk_div | PWM_OUTPUT_ENABLE);
-
- /* enable back */
- crc_pwm_enable(c, pwm);
}
/* change the pwm duty cycle */
--
2.28.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 03/16] pwm: lpss: Fix off by one error in base_unit math in pwm_lpss_prepare()
According to the data-sheet the way the PWM controller works is that
each input clock-cycle the base_unit gets added to a N bit counter and
that counter overflowing determines the PWM output frequency.
So assuming e.g. a 16 bit counter this means that if base_unit is set to 1,
after 65535 input clock-cycles the counter has been increased from 0 to
65535 and it will overflow on the next cycle, so it will overflow after
every 65536 clock cycles and thus the calculations done in
pwm_lpss_prepare() should use 65536 and not 65535.
This commit fixes this. Note this also aligns the calculations in
pwm_lpss_prepare() with those in pwm_lpss_get_state().
Note this effectively reverts commit 684309e5043e ("pwm: lpss: Avoid
potential overflow of base_unit"). The next patch in this series really
fixes the potential overflow of the base_unit value.
Fixes: 684309e5043e ("pwm: lpss: Avoid potential overflow of base_unit")
Reviewed-by: Andy Shevchenko
Acked-by: Uwe Kleine-König
Signed-off-by: Hans de Goede
---
Changes in v3:
- Add Fixes tag
- Add Reviewed-by: Andy Shevchenko tag
---
drivers/pwm/pwm-lpss.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c
index 9d965ffe66d1..43b1fc634af1 100644
--- a/drivers/pwm/pwm-lpss.c
+++ b/drivers/pwm/pwm-lpss.c
@@ -93,7 +93,7 @@ static void pwm_lpss_prepare(struct pwm_lpss_chip *lpwm,
struct pwm_device *pwm,
* The equation is:
* base_unit = round(base_unit_range * freq / c)
*/
- base_unit_range = BIT(lpwm->info->base_unit_bits) - 1;
+ base_unit_range = BIT(lpwm->info->base_unit_bits);
freq *= base_unit_range;
base_unit = DIV_ROUND_CLOSEST_ULL(freq, c);
@@ -104,8 +104,8 @@ static void pwm_lpss_prepare(struct pwm_lpss_chip *lpwm,
struct pwm_device *pwm,
orig_ctrl = ctrl = pwm_lpss_read(pwm);
ctrl &= ~PWM_ON_TIME_DIV_MASK;
- ctrl &= ~(base_unit_range << PWM_BASE_UNIT_SHIFT);
- base_unit &= base_unit_range;
+ ctrl &= ~((base_unit_range - 1) << PWM_BASE_UNIT_SHIFT);
+ base_unit &= (base_unit_range - 1);
ctrl |= (u32) base_unit << PWM_BASE_UNIT_SHIFT;
ctrl |= on_time_div;
--
2.28.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 08/16] pwm: crc: Fix off-by-one error in the clock-divider calculations
The CRC PWM controller has a clock-divider which divides the clock with
a value between 1-128. But as can seen from the PWM_DIV_CLK_xxx
defines, this range maps to a register value of 0-127.
So after calculating the clock-divider we must subtract 1 to get the
register value, unless the requested frequency was so high that the
calculation has already resulted in a (rounded) divider value of 0.
Note that before this fix, setting a period of PWM_MAX_PERIOD_NS which
corresponds to the max. divider value of 128 could have resulted in a
bug where the code would use 128 as divider-register value which would
have resulted in an actual divider value of 0 (and the enable bit being
set). A rounding error stopped this bug from actually happen. This
same rounding error means that after the subtraction of 1 it is impossible
to set the divider to 128. Also bump PWM_MAX_PERIOD_NS by 1 ns to allow
setting a divider of 128 (register-value 127).
Reviewed-by: Andy Shevchenko
Signed-off-by: Hans de Goede
---
Changes in v3:
- Introduce crc_pwm_calc_clk_div() here instead of later in the patch-set
to reduce the amount of churn in the patch-set a bit
---
drivers/pwm/pwm-crc.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-crc.c b/drivers/pwm/pwm-crc.c
index c056eb9b858c..44ec7d5b63e1 100644
--- a/drivers/pwm/pwm-crc.c
+++ b/drivers/pwm/pwm-crc.c
@@ -22,7 +22,7 @@
#define PWM_MAX_LEVEL 0xFF
#define PWM_BASE_CLK_MHZ 6 /* 6 MHz */
-#define PWM_MAX_PERIOD_NS 5461333 /* 183 Hz */
+#define PWM_MAX_PERIOD_NS 5461334 /* 183 Hz */
/**
* struct crystalcove_pwm - Crystal Cove PWM controller
@@ -39,6 +39,18 @@ static inline struct crystalcove_pwm *to_crc_pwm(struct
pwm_chip *pc)
return container_of(pc, struct crystalcove_pwm, chip);
}
+static int crc_pwm_calc_clk_div(int period_ns)
+{
+ int clk_div;
+
+ clk_div = PWM_BASE_CLK_MHZ * period_ns / (256 * NSEC_PER_USEC);
+ /* clk_div 1 - 128, maps to register values 0-127 */
+ if (clk_div > 0)
+ clk_div--;
+
+ return clk_div;
+}
+
static int crc_pwm_enable(struct pwm_chip *c, struct pwm_device *pwm)
{
struct crystalcove_pwm *crc_pwm = to_crc_pwm(c);
@@ -68,11 +80,10 @@ static int crc_pwm_config(struct pwm_chip *c, struct
pwm_device *pwm,
}
if (pwm_get_period(pwm) != period_ns) {
- int clk_div;
+ int clk_div = crc_pwm_calc_clk_div(period_ns);
/* changing the clk divisor, need to disable fisrt */
crc_pwm_disable(c, pwm);
- clk_div = PWM_BASE_CLK_MHZ * period_ns / (256 * NSEC_PER_USEC);
regmap_write(crc_pwm->regmap, PWM0_CLK_DIV,
clk_div | PWM_OUTPUT_ENABLE);
--
2.28.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 01/16] ACPI / LPSS: Resume Cherry Trail PWM controller in no-irq phase
The DSDTs on most Cherry Trail devices have an ugly clutch where the PWM
controller gets poked from the _PS0 method of the graphics-card device:
Local0 = PSAT /* \_SB_.PCI0.GFX0.PSAT */
If (((Local0 & 0x03) == 0x03))
{
PSAT &= 0xFFFC
Local1 = PSAT /* \_SB_.PCI0.GFX0.PSAT */
RSTA = Zero
RSTF = Zero
RSTA = One
RSTF = One
PWMB |= 0xC000
PWMC = PWMB /* \_SB_.PCI0.GFX0.PWMB */
}
Where PSAT is the power-status register of the PWM controller, so if it
is in D3 when the GFX0 device's PS0 method runs then it will turn it on
and restore the PWM ctrl register value it saved from its PS3 handler.
Note not only does it restore it, it ors it with 0xC000 turning it
on at a time where we may not want it to get turned on at all.
The pwm_get call which the i915 driver does to get a reference to the
PWM controller, already adds a device-link making the GFX0 device a
consumer of the PWM device. So it should already have been resumed when
the above AML runs and the AML should thus not do its undesirable poking
of the PWM controller register.
But the PCI core powers on PCI devices in the no-irq resume phase and
thus calls the troublesome PS0 method in the no-irq resume phase.
Where as LPSS devices by default are resumed in the early resume phase.
This commit sets the resume_from_noirq flag in the bsw_pwm_dev_desc
struct, so that Cherry Trail PWM controllers will be resumed in the
no-irq phase. Together with the device-link added by the pwm-get this
ensures that the PWM controller will be on when the troublesome PS0
method runs, which stops it from poking the PWM controller.
Acked-by: Rafael J. Wysocki
Signed-off-by: Hans de Goede
---
drivers/acpi/acpi_lpss.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
index 5e2bfbcf526f..67892fc0b822 100644
--- a/drivers/acpi/acpi_lpss.c
+++ b/drivers/acpi/acpi_lpss.c
@@ -257,6 +257,7 @@ static const struct lpss_device_desc bsw_pwm_dev_desc = {
.flags = LPSS_SAVE_CTX | LPSS_NO_D3_DELAY,
.prv_offset = 0x800,
.setup = bsw_pwm_setup,
+ .resume_from_noirq = true,
};
static const struct lpss_device_desc byt_uart_dev_desc = {
--
2.28.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 11/16] pwm: crc: Implement apply() method to support the new atomic PWM API
Replace the enable, disable and config pwm_ops with an apply op,
to support the new atomic PWM API.
Reviewed-by: Andy Shevchenko
Signed-off-by: Hans de Goede
---
Changes in v6:
- Rebase on 5.9-rc1
- Use do_div when calculating level because pwm_state.period and .duty_cycle
are now u64
Changes in v3:
- Keep crc_pwm_calc_clk_div() helper to avoid needless churn
---
drivers/pwm/pwm-crc.c | 89 ++-
1 file changed, 54 insertions(+), 35 deletions(-)
diff --git a/drivers/pwm/pwm-crc.c b/drivers/pwm/pwm-crc.c
index b72008c9b072..27dc30882424 100644
--- a/drivers/pwm/pwm-crc.c
+++ b/drivers/pwm/pwm-crc.c
@@ -51,59 +51,78 @@ static int crc_pwm_calc_clk_div(int period_ns)
return clk_div;
}
-static int crc_pwm_enable(struct pwm_chip *c, struct pwm_device *pwm)
+static int crc_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+const struct pwm_state *state)
{
- struct crystalcove_pwm *crc_pwm = to_crc_pwm(c);
- int div = crc_pwm_calc_clk_div(pwm_get_period(pwm));
+ struct crystalcove_pwm *crc_pwm = to_crc_pwm(chip);
+ struct device *dev = crc_pwm->chip.dev;
+ int err;
- regmap_write(crc_pwm->regmap, PWM0_CLK_DIV, div | PWM_OUTPUT_ENABLE);
- regmap_write(crc_pwm->regmap, BACKLIGHT_EN, 1);
+ if (state->period > PWM_MAX_PERIOD_NS) {
+ dev_err(dev, "un-supported period_ns\n");
+ return -EINVAL;
+ }
- return 0;
-}
+ if (state->polarity != PWM_POLARITY_NORMAL)
+ return -EOPNOTSUPP;
-static void crc_pwm_disable(struct pwm_chip *c, struct pwm_device *pwm)
-{
- struct crystalcove_pwm *crc_pwm = to_crc_pwm(c);
- int div = crc_pwm_calc_clk_div(pwm_get_period(pwm));
+ if (pwm_is_enabled(pwm) && !state->enabled) {
+ err = regmap_write(crc_pwm->regmap, BACKLIGHT_EN, 0);
+ if (err) {
+ dev_err(dev, "Error writing BACKLIGHT_EN %d\n", err);
+ return err;
+ }
+ }
- regmap_write(crc_pwm->regmap, BACKLIGHT_EN, 0);
- regmap_write(crc_pwm->regmap, PWM0_CLK_DIV, div);
-}
+ if (pwm_get_duty_cycle(pwm) != state->duty_cycle ||
+ pwm_get_period(pwm) != state->period) {
+ u64 level = state->duty_cycle * PWM_MAX_LEVEL;
-static int crc_pwm_config(struct pwm_chip *c, struct pwm_device *pwm,
- int duty_ns, int period_ns)
-{
- struct crystalcove_pwm *crc_pwm = to_crc_pwm(c);
- struct device *dev = crc_pwm->chip.dev;
- int level;
+ do_div(level, state->period);
- if (period_ns > PWM_MAX_PERIOD_NS) {
- dev_err(dev, "un-supported period_ns\n");
- return -EINVAL;
+ err = regmap_write(crc_pwm->regmap, PWM0_DUTY_CYCLE, level);
+ if (err) {
+ dev_err(dev, "Error writing PWM0_DUTY_CYCLE %d\n", err);
+ return err;
+ }
}
- if (pwm_get_period(pwm) != period_ns) {
- int clk_div = crc_pwm_calc_clk_div(period_ns);
-
+ if (pwm_is_enabled(pwm) && state->enabled &&
+ pwm_get_period(pwm) != state->period) {
/* changing the clk divisor, clear PWM_OUTPUT_ENABLE first */
- regmap_write(crc_pwm->regmap, PWM0_CLK_DIV, 0);
+ err = regmap_write(crc_pwm->regmap, PWM0_CLK_DIV, 0);
+ if (err) {
+ dev_err(dev, "Error writing PWM0_CLK_DIV %d\n", err);
+ return err;
+ }
+ }
- regmap_write(crc_pwm->regmap, PWM0_CLK_DIV,
- clk_div | PWM_OUTPUT_ENABLE);
+ if (pwm_get_period(pwm) != state->period ||
+ pwm_is_enabled(pwm) != state->enabled) {
+ int clk_div = crc_pwm_calc_clk_div(state->period);
+ int pwm_output_enable = state->enabled ? PWM_OUTPUT_ENABLE : 0;
+
+ err = regmap_write(crc_pwm->regmap, PWM0_CLK_DIV,
+ clk_div | pwm_output_enable);
+ if (err) {
+ dev_err(dev, "Error writing PWM0_CLK_DIV %d\n", err);
+ return err;
+ }
}
- /* change the pwm duty cycle */
- level = duty_ns * PWM_MAX_LEVEL / period_ns;
- regmap_write(crc_pwm->regmap, PWM0_DUTY_CYCLE, level);
+ if (!pwm_is_enabled(pwm) && state->enabled) {
+ err = regmap_write(crc_pwm->regmap, BACKLIGHT_EN, 1);
+ if (err) {
+ dev_err(dev, "Error writing BACKLIGHT_EN %d\n", err);
+ return err;
+ }
+ }
return 0;
}
static const struct pwm_ops crc_pwm_ops = {
- .config = crc_pwm_config,
- .enable = crc_pwm_enable,
- .disable = crc_pwm_disable,
+ .apply = crc_pwm_apply
[Intel-gfx] [PATCH v6 10/16] pwm: crc: Enable/disable PWM output on enable/disable
The pwm-crc code is using 2 different enable bits:
1. bit 7 of the PWM0_CLK_DIV (PWM_OUTPUT_ENABLE)
2. bit 0 of the BACKLIGHT_EN register
So far we've kept the PWM_OUTPUT_ENABLE bit set when disabling the PWM,
this commit makes crc_pwm_disable() clear it on disable and makes
crc_pwm_enable() set it again on re-enable.
Acked-by: Uwe Kleine-König
Reviewed-by: Andy Shevchenko
Signed-off-by: Hans de Goede
---
Changes in v3:
- Remove paragraph about tri-stating the output from the commit message,
we don't have a datasheet so this was just an unfounded guess
---
drivers/pwm/pwm-crc.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/pwm/pwm-crc.c b/drivers/pwm/pwm-crc.c
index 81232da0c767..b72008c9b072 100644
--- a/drivers/pwm/pwm-crc.c
+++ b/drivers/pwm/pwm-crc.c
@@ -54,7 +54,9 @@ static int crc_pwm_calc_clk_div(int period_ns)
static int crc_pwm_enable(struct pwm_chip *c, struct pwm_device *pwm)
{
struct crystalcove_pwm *crc_pwm = to_crc_pwm(c);
+ int div = crc_pwm_calc_clk_div(pwm_get_period(pwm));
+ regmap_write(crc_pwm->regmap, PWM0_CLK_DIV, div | PWM_OUTPUT_ENABLE);
regmap_write(crc_pwm->regmap, BACKLIGHT_EN, 1);
return 0;
@@ -63,8 +65,10 @@ static int crc_pwm_enable(struct pwm_chip *c, struct
pwm_device *pwm)
static void crc_pwm_disable(struct pwm_chip *c, struct pwm_device *pwm)
{
struct crystalcove_pwm *crc_pwm = to_crc_pwm(c);
+ int div = crc_pwm_calc_clk_div(pwm_get_period(pwm));
regmap_write(crc_pwm->regmap, BACKLIGHT_EN, 0);
+ regmap_write(crc_pwm->regmap, PWM0_CLK_DIV, div);
}
static int crc_pwm_config(struct pwm_chip *c, struct pwm_device *pwm,
--
2.28.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 12/16] pwm: crc: Implement get_state() method
Implement the pwm_ops.get_state() method to complete the support for the
new atomic PWM API.
Reviewed-by: Andy Shevchenko
Signed-off-by: Hans de Goede
---
Changes in v6:
- Rebase on 5.9-rc1
- Use DIV_ROUND_UP_ULL because pwm_state.period and .duty_cycle are now u64
Changes in v5:
- Fix an indentation issue
Changes in v4:
- Use DIV_ROUND_UP when calculating the period and duty_cycle from the
controller's register values
Changes in v3:
- Add Andy's Reviewed-by tag
- Remove extra whitespace to align some code after assignments (requested by
Uwe Kleine-König)
---
drivers/pwm/pwm-crc.c | 31 +++
1 file changed, 31 insertions(+)
diff --git a/drivers/pwm/pwm-crc.c b/drivers/pwm/pwm-crc.c
index 27dc30882424..ecfdfac0c2d9 100644
--- a/drivers/pwm/pwm-crc.c
+++ b/drivers/pwm/pwm-crc.c
@@ -121,8 +121,39 @@ static int crc_pwm_apply(struct pwm_chip *chip, struct
pwm_device *pwm,
return 0;
}
+static void crc_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
+ struct pwm_state *state)
+{
+ struct crystalcove_pwm *crc_pwm = to_crc_pwm(chip);
+ struct device *dev = crc_pwm->chip.dev;
+ unsigned int clk_div, clk_div_reg, duty_cycle_reg;
+ int error;
+
+ error = regmap_read(crc_pwm->regmap, PWM0_CLK_DIV, &clk_div_reg);
+ if (error) {
+ dev_err(dev, "Error reading PWM0_CLK_DIV %d\n", error);
+ return;
+ }
+
+ error = regmap_read(crc_pwm->regmap, PWM0_DUTY_CYCLE, &duty_cycle_reg);
+ if (error) {
+ dev_err(dev, "Error reading PWM0_DUTY_CYCLE %d\n", error);
+ return;
+ }
+
+ clk_div = (clk_div_reg & ~PWM_OUTPUT_ENABLE) + 1;
+
+ state->period =
+ DIV_ROUND_UP(clk_div * NSEC_PER_USEC * 256, PWM_BASE_CLK_MHZ);
+ state->duty_cycle =
+ DIV_ROUND_UP_ULL(duty_cycle_reg * state->period, PWM_MAX_LEVEL);
+ state->polarity = PWM_POLARITY_NORMAL;
+ state->enabled = !!(clk_div_reg & PWM_OUTPUT_ENABLE);
+}
+
static const struct pwm_ops crc_pwm_ops = {
.apply = crc_pwm_apply,
+ .get_state = crc_pwm_get_state,
};
static int crystalcove_pwm_probe(struct platform_device *pdev)
--
2.28.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 02/16] ACPI / LPSS: Save Cherry Trail PWM ctx registers only once (at activation)
The DSDTs on most Cherry Trail devices have an ugly clutch where the PWM
controller gets turned off from the _PS3 method of the graphics-card dev:
Method (_PS3, 0, Serialized) // _PS3: Power State 3
{
...
PWMB = PWMC /* \_SB_.PCI0.GFX0.PWMC */
PSAT |= 0x03
Local0 = PSAT /* \_SB_.PCI0.GFX0.PSAT */
...
}
Where PSAT is the power-status register of the PWM controller.
Since the i915 driver will do a pwm_get on the pwm device as it uses it to
control the LCD panel backlight, there is a device-link marking the i915
device as a consumer of the pwm device. So that the PWM controller will
always be suspended after the i915 driver suspends (which is the right
thing to do). This causes the above GFX0 PS3 AML code to run before
acpi_lpss.c calls acpi_lpss_save_ctx().
So on these devices the PWM controller will already be off when
acpi_lpss_save_ctx() runs. This causes it to read/save all 1-s (0x)
as ctx register values.
When these bogus values get restored on resume the PWM controller actually
keeps working, since most bits are reserved, but this does set bit 3 of
the LPSS General purpose register, which for the PWM controller has the
following function: "This bit is re-used to support 32kHz slow mode.
Default is 19.2MHz as PWM source clock".
This causes the clock of the PWM controller to switch from 19.2MHz to
32KHz, which is a slow-down of a factor 600. Surprisingly enough so far
there have been few bug reports about this. This is likely because the
i915 driver was hardcoding the PWM frequency to 46 KHz, which divided
by 600 would result in a PWM frequency of approx. 78 Hz, which mostly
still works fine. There are some bug reports about the LCD backlight
flickering after suspend/resume which are likely caused by this issue.
But with the upcoming patch-series to finally switch the i915 drivers
code for external PWM controllers to use the atomic API and to honor
the PWM frequency specified in the video BIOS (VBT), this becomes a much
bigger problem. On most cases the VBT specifies either 200 Hz or 20
KHz as PWM frequency, which with the mentioned issue ends up being either
1/3 Hz, where the backlight actually visible blinks on and off every 3s,
or in 33 Hz and horrible flickering of the backlight.
There are a number of possible solutions to this problem:
1. Make acpi_lpss_save_ctx() run before GFX0._PS3
Pro: Clean solution from pov of not medling with save/restore ctx code
Con: As mentioned the current ordering is the right thing to do
Con: Requires assymmetry in at what suspend/resume phase we do the save vs
restore, requiring more suspend/resume ordering hacks in already
convoluted acpi_lpss.c suspend/resume code.
2. Do some sort of save once mode for the LPSS ctx
Pro: Reasonably clean
Con: Needs a new LPSS flag + code changes to handle the flag
3. Detect we have failed to save the ctx registers and do not restore them
Pro: Not PWM specific, might help with issues on other LPSS devices too
Con: If we can get away with not restoring the ctx why bother with it at
all?
4. Do not save the ctx for CHT PWM controllers
Pro: Clean, as simple as dropping a flag?
Con: Not so simple as dropping a flag, needs a new flag to ensure that
we still do lpss_deassert_reset() on device activation.
5. Make the pwm-lpss code fixup the LPSS-context registers
Pro: Keeps acpi_lpss.c code clean
Con: Moves knowledge of LPSS-context into the pwm-lpss.c code
1 and 5 both do not seem to be a desirable way forward.
3 and 4 seem ok, but they both assume that restoring the LPSS-context
registers is not necessary. I have done a couple of test and those do
show that restoring the LPSS-context indeed does not seem to be necessary
on devices using s2idle suspend (and successfully reaching S0i3). But I
have no hardware to test deep / S3 suspend. So I'm not sure that not
restoring the context is safe.
That leaves solution 2, which is about as simple / clean as 3 and 4,
so this commit fixes the described problem by implementing a new
LPSS_SAVE_CTX_ONCE flag and setting that for the CHT PWM controllers.
Acked-by: Rafael J. Wysocki
Signed-off-by: Hans de Goede
---
Changes in v2:
- Move #define LPSS_SAVE_CTX_ONCE define to group it with LPSS_SAVE_CTX
---
drivers/acpi/acpi_lpss.c | 21 +
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
index 67892fc0b822..a8d7d83ac761 100644
--- a/drivers/acpi/acpi_lpss.c
+++ b/drivers/acpi/acpi_lpss.c
@@ -67,7 +67,15 @@ ACPI_MODULE_NAME("acpi_lpss");
#define LPSS_CLK_DIVIDER BIT(2)
#define LPSS_LTR BIT(3)
#define LPSS_SAVE_CTX BIT(4)
-#define LPSS_NO_D3_DELAY BIT(5)
+/*
+ * For some devices the DSDT AML code for another device turns off the device
+ * before our s
[Intel-gfx] [PATCH v6 07/16] pwm: crc: Fix period / duty_cycle times being off by a factor of 256
While looking into adding atomic-pwm support to the pwm-crc driver I noticed something odd, there is a PWM_BASE_CLK define of 6 MHz and there is a clock-divider which divides this with a value between 1-128, and there are 256 duty-cycle steps. The pwm-crc code before this commit assumed that a clock-divider setting of 1 means that the PWM output is running at 6 MHZ, if that is true, where do these 256 duty-cycle steps come from? This would require an internal frequency of 256 * 6 MHz = 1.5 GHz, that seems unlikely for a PMIC which is using a silicon process optimized for power-switching transistors. It is way more likely that there is an 8 bit counter for the duty cycle which acts as an extra fixed divider wrt the PWM output frequency. The main user of the pwm-crc driver is the i915 GPU driver which uses it for backlight control. Lets compare the PWM register values set by the video-BIOS (the GOP), assuming the extra fixed divider is present versus the PWM frequency specified in the Video-BIOS-Tables: Device: PWM Hz set by BIOS PWM Hz specified in VBT Asus T100TA 200 200 Asus T100HA 200 200 Lenovo Miix 2 8 23437 2 Toshiba WT8-A 23437 2 So as we can see if we assume the extra division by 256 then the register values set by the GOP are an exact match for the VBT values, where as otherwise the values would be of by a factor of 256. This commit fixes the period / duty_cycle calculations to take the extra division by 256 into account. Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede --- Changes in v3: - Use NSEC_PER_USEC instead of adding a new (non-sensical) NSEC_PER_MHZ define --- drivers/pwm/pwm-crc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/pwm/pwm-crc.c b/drivers/pwm/pwm-crc.c index 272eeb071147..c056eb9b858c 100644 --- a/drivers/pwm/pwm-crc.c +++ b/drivers/pwm/pwm-crc.c @@ -21,8 +21,8 @@ #define PWM_MAX_LEVEL 0xFF -#define PWM_BASE_CLK 600 /* 6 MHz */ -#define PWM_MAX_PERIOD_NS 21333/* 46.875KHz */ +#define PWM_BASE_CLK_MHZ 6 /* 6 MHz */ +#define PWM_MAX_PERIOD_NS 5461333 /* 183 Hz */ /** * struct crystalcove_pwm - Crystal Cove PWM controller @@ -72,7 +72,7 @@ static int crc_pwm_config(struct pwm_chip *c, struct pwm_device *pwm, /* changing the clk divisor, need to disable fisrt */ crc_pwm_disable(c, pwm); - clk_div = PWM_BASE_CLK * period_ns / NSEC_PER_SEC; + clk_div = PWM_BASE_CLK_MHZ * period_ns / (256 * NSEC_PER_USEC); regmap_write(crc_pwm->regmap, PWM0_CLK_DIV, clk_div | PWM_OUTPUT_ENABLE); -- 2.28.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 13/16] drm/i915: panel: Add get_vbt_pwm_freq() helper
Factor the code which checks and drm_dbg_kms-s the VBT PWM frequency
out of get_backlight_max_vbt().
This is a preparation patch for honering the VBT PWM frequency for
devices which use an external PWM controller (devices using
pwm_setup_backlight()).
Acked-by: Jani Nikula
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/i915/display/intel_panel.c | 27 ++
1 file changed, 17 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_panel.c
b/drivers/gpu/drm/i915/display/intel_panel.c
index bbde3b12c311..ec6b9d704542 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.c
+++ b/drivers/gpu/drm/i915/display/intel_panel.c
@@ -1543,18 +1543,9 @@ static u32 vlv_hz_to_pwm(struct intel_connector
*connector, u32 pwm_freq_hz)
return DIV_ROUND_CLOSEST(clock, pwm_freq_hz * mul);
}
-static u32 get_backlight_max_vbt(struct intel_connector *connector)
+static u16 get_vbt_pwm_freq(struct drm_i915_private *dev_priv)
{
- struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
- struct intel_panel *panel = &connector->panel;
u16 pwm_freq_hz = dev_priv->vbt.backlight.pwm_freq_hz;
- u32 pwm;
-
- if (!panel->backlight.hz_to_pwm) {
- drm_dbg_kms(&dev_priv->drm,
- "backlight frequency conversion not supported\n");
- return 0;
- }
if (pwm_freq_hz) {
drm_dbg_kms(&dev_priv->drm,
@@ -1567,6 +1558,22 @@ static u32 get_backlight_max_vbt(struct intel_connector
*connector)
pwm_freq_hz);
}
+ return pwm_freq_hz;
+}
+
+static u32 get_backlight_max_vbt(struct intel_connector *connector)
+{
+ struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
+ struct intel_panel *panel = &connector->panel;
+ u16 pwm_freq_hz = get_vbt_pwm_freq(dev_priv);
+ u32 pwm;
+
+ if (!panel->backlight.hz_to_pwm) {
+ drm_dbg_kms(&dev_priv->drm,
+ "backlight frequency conversion not supported\n");
+ return 0;
+ }
+
pwm = panel->backlight.hz_to_pwm(connector, pwm_freq_hz);
if (!pwm) {
drm_dbg_kms(&dev_priv->drm,
--
2.28.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 16/16] drm/i915: panel: Use atomic PWM API for devs with an external PWM controller
Now that the PWM drivers which we use have been converted to the atomic
PWM API, we can move the i915 panel code over to using the atomic PWM API.
The removes a long standing FIXME and this removes a flicker where
the backlight brightness would jump to 100% when i915 loads even if
using the fastset path.
Note that this commit also simplifies pwm_disable_backlight(), by dropping
the intel_panel_actually_set_backlight(..., 0) call. This call sets the
PWM to 0% duty-cycle. I believe that this call was only present as a
workaround for a bug in the pwm-crc.c driver where it failed to clear the
PWM_OUTPUT_ENABLE bit. This is fixed by an earlier patch in this series.
After the dropping of this workaround, the usleep call, which seems
unnecessary to begin with, has no useful effect anymore, so drop that too.
Acked-by: Jani Nikula
Signed-off-by: Hans de Goede
---
Changes in v6:
- Drop the pwm_get_period() check in pwm_setup(), it is now longer needed
now that we use pwm_get_relative_duty_cycle()
Changes in v4:
- Add a note to the commit message about the dropping of the
intel_panel_actually_set_backlight() and usleep() calls from
pwm_disable_backlight()
- Use the pwm_set/get_relative_duty_cycle() helpers instead of using DIY code
for this
---
.../drm/i915/display/intel_display_types.h| 3 +-
drivers/gpu/drm/i915/display/intel_panel.c| 70 ---
2 files changed, 33 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 7171e7c8d928..e3ebe7c313ba 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -28,6 +28,7 @@
#include
#include
+#include
#include
#include
@@ -223,7 +224,7 @@ struct intel_panel {
bool util_pin_active_low; /* bxt+ */
u8 controller; /* bxt+ only */
struct pwm_device *pwm;
- int pwm_period_ns;
+ struct pwm_state pwm_state;
/* DPCD backlight */
u8 pwmgen_bit_count;
diff --git a/drivers/gpu/drm/i915/display/intel_panel.c
b/drivers/gpu/drm/i915/display/intel_panel.c
index 6d27630dd1e4..f2b725f85f99 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.c
+++ b/drivers/gpu/drm/i915/display/intel_panel.c
@@ -592,10 +592,10 @@ static u32 bxt_get_backlight(struct intel_connector
*connector)
static u32 pwm_get_backlight(struct intel_connector *connector)
{
struct intel_panel *panel = &connector->panel;
- int duty_ns;
+ struct pwm_state state;
- duty_ns = pwm_get_duty_cycle(panel->backlight.pwm);
- return DIV_ROUND_UP(duty_ns * 100, panel->backlight.pwm_period_ns);
+ pwm_get_state(panel->backlight.pwm, &state);
+ return pwm_get_relative_duty_cycle(&state, 100);
}
static void lpt_set_backlight(const struct drm_connector_state *conn_state,
u32 level)
@@ -669,10 +669,9 @@ static void bxt_set_backlight(const struct
drm_connector_state *conn_state, u32
static void pwm_set_backlight(const struct drm_connector_state *conn_state,
u32 level)
{
struct intel_panel *panel =
&to_intel_connector(conn_state->connector)->panel;
- int duty_ns = DIV_ROUND_UP(level * panel->backlight.pwm_period_ns, 100);
- pwm_config(panel->backlight.pwm, duty_ns,
- panel->backlight.pwm_period_ns);
+ pwm_set_relative_duty_cycle(&panel->backlight.pwm_state, level, 100);
+ pwm_apply_state(panel->backlight.pwm, &panel->backlight.pwm_state);
}
static void
@@ -841,10 +840,8 @@ static void pwm_disable_backlight(const struct
drm_connector_state *old_conn_sta
struct intel_connector *connector =
to_intel_connector(old_conn_state->connector);
struct intel_panel *panel = &connector->panel;
- /* Disable the backlight */
- intel_panel_actually_set_backlight(old_conn_state, 0);
- usleep_range(2000, 3000);
- pwm_disable(panel->backlight.pwm);
+ panel->backlight.pwm_state.enabled = false;
+ pwm_apply_state(panel->backlight.pwm, &panel->backlight.pwm_state);
}
void intel_panel_disable_backlight(const struct drm_connector_state
*old_conn_state)
@@ -1176,9 +1173,12 @@ static void pwm_enable_backlight(const struct
intel_crtc_state *crtc_state,
{
struct intel_connector *connector =
to_intel_connector(conn_state->connector);
struct intel_panel *panel = &connector->panel;
+ int level = panel->backlight.level;
- pwm_enable(panel->backlight.pwm);
- intel_panel_actually_set_backlight(conn_state, panel->backlight.level);
+ level = intel_panel_compute_brightness(connector, level);
+ pwm_set_relative_duty_cycle(&panel->backlight.pwm_state, level, 100);
+ panel->backlight.pwm_state.enabled = true;
+ pwm_apply_state(panel->backlight.pwm, &panel->backlight.pwm_state);
}
static void __intel_panel_e
[Intel-gfx] [PATCH v6 15/16] drm/i915: panel: Honor the VBT PWM min setting for devs with an external PWM controller
So far for devices using an external PWM controller (devices using pwm_setup_backlight()), we have been hardcoding the minimum allowed PWM level to 0. But several of these devices specify a non 0 minimum setting in their VBT. Change pwm_setup_backlight() to use get_backlight_min_vbt() to get the minimum level. Acked-by: Jani Nikula Signed-off-by: Hans de Goede --- drivers/gpu/drm/i915/display/intel_panel.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c index 7fb162fac8a1..6d27630dd1e4 100644 --- a/drivers/gpu/drm/i915/display/intel_panel.c +++ b/drivers/gpu/drm/i915/display/intel_panel.c @@ -1925,8 +1925,8 @@ static int pwm_setup_backlight(struct intel_connector *connector, */ pwm_apply_args(panel->backlight.pwm); - panel->backlight.min = 0; /* 0% */ panel->backlight.max = 100; /* 100% */ + panel->backlight.min = get_backlight_min_vbt(connector); level = intel_panel_compute_brightness(connector, 100); ns = DIV_ROUND_UP(level * panel->backlight.pwm_period_ns, 100); @@ -1941,8 +1941,9 @@ static int pwm_setup_backlight(struct intel_connector *connector, level = DIV_ROUND_UP_ULL(pwm_get_duty_cycle(panel->backlight.pwm) * 100, panel->backlight.pwm_period_ns); - panel->backlight.level = - intel_panel_compute_brightness(connector, level); + level = intel_panel_compute_brightness(connector, level); + panel->backlight.level = clamp(level, panel->backlight.min, + panel->backlight.max); panel->backlight.enabled = panel->backlight.level != 0; drm_info(&dev_priv->drm, "Using %s PWM for LCD backlight control\n", -- 2.28.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 14/16] drm/i915: panel: Honor the VBT PWM frequency for devs with an external PWM controller
So far for devices using an external PWM controller (devices using
pwm_setup_backlight()), we have been hardcoding the period-time passed to
pwm_config() to 21333 ns.
I suspect this was done because many VBTs set the PWM frequency to 200
which corresponds to a period-time of 500 ns, which greatly exceeds
the PWM_MAX_PERIOD_NS define in the Crystal Cove PMIC PWM driver, which
used to be 21333.
This PWM_MAX_PERIOD_NS define was actually based on a bug in the PWM
driver where its period and duty-cycle times where off by a factor of 256.
Due to this bug the hardcoded CRC_PMIC_PWM_PERIOD_NS value of 21333 would
result in the PWM driver using its divider of 128, which would result in
a PWM output frequency of 600 Hz / 256 / 128 = 183 Hz. So actually
pretty close to the default VBT value of 200 Hz.
Now that this bug in the pwm-crc driver is fixed, we can actually use
the VBT defined frequency.
This is important because:
a) With the pwm-crc driver fixed it will now translate the hardcoded
CRC_PMIC_PWM_PERIOD_NS value of 21333 ns / 46 Khz to a PWM output
frequency of 23 KHz (the max it can do).
b) The pwm-lpss driver used on many models has always honored the
21333 ns / 46 Khz request
Some panels do not like such high output frequencies. E.g. on a Terra
Pad 1061 tablet, using the LPSS PWM controller, the backlight would go
from off to max, when changing the sysfs backlight brightness value from
90-100%, anything under aprox. 90% would turn the backlight fully off.
Honoring the VBT specified PWM frequency will also hopefully fix the
various bug reports which we have received about users perceiving the
backlight to flicker after a suspend/resume cycle.
Acked-by: Jani Nikula
Signed-off-by: Hans de Goede
---
.../drm/i915/display/intel_display_types.h| 1 +
drivers/gpu/drm/i915/display/intel_panel.c| 19 +++
2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index e8f809161c75..7171e7c8d928 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -223,6 +223,7 @@ struct intel_panel {
bool util_pin_active_low; /* bxt+ */
u8 controller; /* bxt+ only */
struct pwm_device *pwm;
+ int pwm_period_ns;
/* DPCD backlight */
u8 pwmgen_bit_count;
diff --git a/drivers/gpu/drm/i915/display/intel_panel.c
b/drivers/gpu/drm/i915/display/intel_panel.c
index ec6b9d704542..7fb162fac8a1 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.c
+++ b/drivers/gpu/drm/i915/display/intel_panel.c
@@ -40,8 +40,6 @@
#include "intel_dsi_dcs_backlight.h"
#include "intel_panel.h"
-#define CRC_PMIC_PWM_PERIOD_NS 21333
-
void
intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
struct drm_display_mode *adjusted_mode)
@@ -597,7 +595,7 @@ static u32 pwm_get_backlight(struct intel_connector
*connector)
int duty_ns;
duty_ns = pwm_get_duty_cycle(panel->backlight.pwm);
- return DIV_ROUND_UP(duty_ns * 100, CRC_PMIC_PWM_PERIOD_NS);
+ return DIV_ROUND_UP(duty_ns * 100, panel->backlight.pwm_period_ns);
}
static void lpt_set_backlight(const struct drm_connector_state *conn_state,
u32 level)
@@ -671,9 +669,10 @@ static void bxt_set_backlight(const struct
drm_connector_state *conn_state, u32
static void pwm_set_backlight(const struct drm_connector_state *conn_state,
u32 level)
{
struct intel_panel *panel =
&to_intel_connector(conn_state->connector)->panel;
- int duty_ns = DIV_ROUND_UP(level * CRC_PMIC_PWM_PERIOD_NS, 100);
+ int duty_ns = DIV_ROUND_UP(level * panel->backlight.pwm_period_ns, 100);
- pwm_config(panel->backlight.pwm, duty_ns, CRC_PMIC_PWM_PERIOD_NS);
+ pwm_config(panel->backlight.pwm, duty_ns,
+ panel->backlight.pwm_period_ns);
}
static void
@@ -1917,6 +1916,9 @@ static int pwm_setup_backlight(struct intel_connector
*connector,
return -ENODEV;
}
+ panel->backlight.pwm_period_ns = NSEC_PER_SEC /
+get_vbt_pwm_freq(dev_priv);
+
/*
* FIXME: pwm_apply_args() should be removed when switching to
* the atomic PWM API.
@@ -1926,9 +1928,10 @@ static int pwm_setup_backlight(struct intel_connector
*connector,
panel->backlight.min = 0; /* 0% */
panel->backlight.max = 100; /* 100% */
level = intel_panel_compute_brightness(connector, 100);
- ns = DIV_ROUND_UP(level * CRC_PMIC_PWM_PERIOD_NS, 100);
+ ns = DIV_ROUND_UP(level * panel->backlight.pwm_period_ns, 100);
- retval = pwm_config(panel->backlight.pwm, ns, CRC_PMIC_PWM_PERIOD_NS);
+ retval = pwm_config(panel->backlight.pwm, ns,
+ panel->backlight.pwm_period_ns);
i
[Intel-gfx] [PATCH v6 06/16] pwm: lpss: Use pwm_lpss_apply() when restoring state on resume
Before this commit a suspend + resume of the LPSS PWM controller
would result in the controller being reset to its defaults of
output-freq = clock/256, duty-cycle=100%, until someone changes
to the output-freq and/or duty-cycle are made.
This problem has been masked so far because the main consumer
(the i915 driver) was always making duty-cycle changes on resume.
With the conversion of the i915 driver to the atomic PWM API the
driver now only disables/enables the PWM on suspend/resume leaving
the output-freq and duty as is, triggering this problem.
The LPSS PWM controller has a mechanism where the ctrl register value
and the actual base-unit and on-time-div values used are latched. When
software sets the SW_UPDATE bit then at the end of the current PWM cycle,
the new values from the ctrl-register will be latched into the actual
registers, and the SW_UPDATE bit will be cleared.
The problem is that before this commit our suspend/resume handling
consisted of simply saving the PWM ctrl register on suspend and
restoring it on resume, without setting the PWM_SW_UPDATE bit.
When the controller has lost its state over a suspend/resume and thus
has been reset to the defaults, just restoring the register is not
enough. We must also set the SW_UPDATE bit to tell the controller to
latch the restored values into the actual registers.
Fixing this problem is not as simple as just or-ing in the value which
is being restored with SW_UPDATE. If the PWM was enabled before we must
write the new settings + PWM_SW_UPDATE before setting PWM_ENABLE.
We must also wait for PWM_SW_UPDATE to become 0 again and depending on the
model we must do this either before or after the setting of PWM_ENABLE.
All the necessary logic for doing this is already present inside
pwm_lpss_apply(), so instead of duplicating this inside the resume
handler, this commit makes the resume handler use pwm_lpss_apply() to
restore the settings when necessary. This fixes the output-freq and
duty-cycle being reset to their defaults on resume.
Reviewed-by: Andy Shevchenko
Signed-off-by: Hans de Goede
---
Changes in v6:
- Add a pwm_lpss_restore_state() helper for re-applying the PWM state on resume
Changes in v5:
- The changes to pwm_lpss_apply() are much cleaner now thanks to the new
pwm_lpss_prepare_enable() helper.
Changes in v3:
- This replaces the "pwm: lpss: Set SW_UPDATE bit when enabling the PWM"
patch from previous versions of this patch-set, which really was a hack
working around the resume issue which this patch fixes properly.
---
drivers/pwm/pwm-lpss.c | 49 +++---
1 file changed, 46 insertions(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c
index 8a136ba2a583..d77869be053c 100644
--- a/drivers/pwm/pwm-lpss.c
+++ b/drivers/pwm/pwm-lpss.c
@@ -166,6 +166,24 @@ static int pwm_lpss_apply(struct pwm_chip *chip, struct
pwm_device *pwm,
return ret;
}
+/*
+ * This is a mirror of pwm_lpss_apply() without pm_runtime reference handling
+ * for restoring the PWM state on resume.
+ */
+static int pwm_lpss_restore_state(struct pwm_lpss_chip *lpwm,
+ struct pwm_device *pwm,
+ const struct pwm_state *state)
+{
+ int ret = 0;
+
+ if (state->enabled)
+ ret = pwm_lpss_prepare_enable(lpwm, pwm, state,
!pwm_is_enabled(pwm));
+ else if (pwm_is_enabled(pwm))
+ pwm_lpss_write(pwm, pwm_lpss_read(pwm) & ~PWM_ENABLE);
+
+ return ret;
+}
+
static void pwm_lpss_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
struct pwm_state *state)
{
@@ -278,10 +296,35 @@ EXPORT_SYMBOL_GPL(pwm_lpss_suspend);
int pwm_lpss_resume(struct device *dev)
{
struct pwm_lpss_chip *lpwm = dev_get_drvdata(dev);
- int i;
+ struct pwm_state saved_state;
+ struct pwm_device *pwm;
+ int i, ret;
+ u32 ctrl;
- for (i = 0; i < lpwm->info->npwm; i++)
- writel(lpwm->saved_ctrl[i], lpwm->regs + i * PWM_SIZE + PWM);
+ for (i = 0; i < lpwm->info->npwm; i++) {
+ pwm = &lpwm->chip.pwms[i];
+
+ ctrl = pwm_lpss_read(pwm);
+ /* If we did not reach S0i3/S3 the controller keeps its state */
+ if (ctrl == lpwm->saved_ctrl[i])
+ continue;
+
+ /*
+* We cannot just blindly restore the old value here. Since we
+* are changing the settings we must set SW_UPDATE and if the
+* PWM was enabled before we must write the new settings +
+* PWM_SW_UPDATE before setting PWM_ENABLE. We must also wait
+* for PWM_SW_UPDATE to become 0 again and depending on the
+* model we must do this either before or after the setting of
+* PWM_ENABLE.
+*/
+ saved_state = pwm->state;
+ /* Update e
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for acpi/pwm/i915: Convert pwm-crc and i915 driver's PWM code to use the atomic PWM API
== Series Details == Series: acpi/pwm/i915: Convert pwm-crc and i915 driver's PWM code to use the atomic PWM API URL : https://patchwork.freedesktop.org/series/80943/ State : warning == Summary == $ dim checkpatch origin/drm-tip 23eeb608a11a ACPI / LPSS: Resume Cherry Trail PWM controller in no-irq phase 3404311e82b8 ACPI / LPSS: Save Cherry Trail PWM ctx registers only once (at activation) cc49add91e96 pwm: lpss: Fix off by one error in base_unit math in pwm_lpss_prepare() a9f8c291fa0f pwm: lpss: Add range limit check for the base_unit register value a7c9b3968280 pwm: lpss: Add pwm_lpss_prepare_enable() helper -:45: CHECK:BOOL_COMPARISON: Using comparison to false is error prone #45: FILE: drivers/pwm/pwm-lpss.c:137: + pwm_lpss_cond_enable(pwm, enable && lpwm->info->bypass == false); -:50: CHECK:BOOL_COMPARISON: Using comparison to true is error prone #50: FILE: drivers/pwm/pwm-lpss.c:142: + pwm_lpss_cond_enable(pwm, enable && lpwm->info->bypass == true); total: 0 errors, 0 warnings, 2 checks, 67 lines checked 19fbcc493a46 pwm: lpss: Use pwm_lpss_apply() when restoring state on resume 23df6f9340c1 pwm: crc: Fix period / duty_cycle times being off by a factor of 256 a9c78de3ade3 pwm: crc: Fix off-by-one error in the clock-divider calculations 9d3242bf8a3e pwm: crc: Fix period changes not having any effect 8b8e82933140 pwm: crc: Enable/disable PWM output on enable/disable 6a44ff1498a2 pwm: crc: Implement apply() method to support the new atomic PWM API c72a3592f7d9 pwm: crc: Implement get_state() method 1f8f97bf9b4a drm/i915: panel: Add get_vbt_pwm_freq() helper c1026447b3e9 drm/i915: panel: Honor the VBT PWM frequency for devs with an external PWM controller 5e7767931e02 drm/i915: panel: Honor the VBT PWM min setting for devs with an external PWM controller 80c31f95dffd drm/i915: panel: Use atomic PWM API for devs with an external PWM controller ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for acpi/pwm/i915: Convert pwm-crc and i915 driver's PWM code to use the atomic PWM API
== Series Details == Series: acpi/pwm/i915: Convert pwm-crc and i915 driver's PWM code to use the atomic PWM API URL : https://patchwork.freedesktop.org/series/80943/ State : success == Summary == CI Bug Log - changes from CI_DRM_8918 -> Patchwork_18393 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18393/index.html Known issues Here are the changes found in Patchwork_18393 that come from known issues: ### IGT changes ### Issues hit * igt@i915_module_load@reload: - fi-byt-j1900: [PASS][1] -> [DMESG-WARN][2] ([i915#1982]) +1 similar issue [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/fi-byt-j1900/igt@i915_module_l...@reload.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18393/fi-byt-j1900/igt@i915_module_l...@reload.html - fi-apl-guc: [PASS][3] -> [DMESG-WARN][4] ([i915#1635] / [i915#1982]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/fi-apl-guc/igt@i915_module_l...@reload.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18393/fi-apl-guc/igt@i915_module_l...@reload.html * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic: - fi-bsw-kefka: [PASS][5] -> [DMESG-WARN][6] ([i915#1982]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/fi-bsw-kefka/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18393/fi-bsw-kefka/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html Possible fixes * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic: - fi-bsw-n3050: [DMESG-WARN][7] ([i915#1982]) -> [PASS][8] +1 similar issue [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/fi-bsw-n3050/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18393/fi-bsw-n3050/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html * igt@kms_flip@basic-flip-vs-wf_vblank@c-hdmi-a2: - fi-skl-guc: [DMESG-WARN][9] ([i915#2203]) -> [PASS][10] [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/fi-skl-guc/igt@kms_flip@basic-flip-vs-wf_vbl...@c-hdmi-a2.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18393/fi-skl-guc/igt@kms_flip@basic-flip-vs-wf_vbl...@c-hdmi-a2.html Warnings * igt@gem_exec_suspend@basic-s0: - fi-kbl-x1275: [DMESG-WARN][11] ([i915#62] / [i915#92]) -> [DMESG-WARN][12] ([i915#62] / [i915#92] / [i915#95]) +3 similar issues [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/fi-kbl-x1275/igt@gem_exec_susp...@basic-s0.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18393/fi-kbl-x1275/igt@gem_exec_susp...@basic-s0.html * igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size: - fi-kbl-x1275: [DMESG-WARN][13] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][14] ([i915#62] / [i915#92]) +2 similar issues [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/fi-kbl-x1275/igt@kms_cursor_leg...@basic-flip-after-cursor-varying-size.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18393/fi-kbl-x1275/igt@kms_cursor_leg...@basic-flip-after-cursor-varying-size.html [i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635 [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982 [i915#2203]: https://gitlab.freedesktop.org/drm/intel/issues/2203 [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62 [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92 [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95 Participating hosts (37 -> 34) -- Missing(3): fi-byt-clapper fi-byt-squawks fi-bsw-cyan Build changes - * Linux: CI_DRM_8918 -> Patchwork_18393 CI-20190529: 20190529 CI_DRM_8918: 7296072da95d2d2550ef2d4836bc72663ee711cc @ git://anongit.freedesktop.org/gfx-ci/linux IGT_5770: f1d0c240ea2e631dfb9f493f37f8fb61cb2b1cf2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_18393: 80c31f95dffd9ca6f68ebccad21ec4a92958ba46 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 80c31f95dffd drm/i915: panel: Use atomic PWM API for devs with an external PWM controller 5e7767931e02 drm/i915: panel: Honor the VBT PWM min setting for devs with an external PWM controller c1026447b3e9 drm/i915: panel: Honor the VBT PWM frequency for devs with an external PWM controller 1f8f97bf9b4a drm/i915: panel: Add get_vbt_pwm_freq() helper c72a3592f7d9 pwm: crc: Implement get_state() method 6a44ff1498a2 pwm: crc: Implement apply() method to support the new atomic PWM API 8b8e82933140 pwm: crc: Enable/disable PWM output on enable/disable 9d3242bf8a3e pwm: crc: Fix period changes not having any e
Re: [Intel-gfx] [PATCH 0/8] Return head pages from find_get_entry and find_lock_entry
> On Aug 19, 2020, at 12:48 PM, Matthew Wilcox (Oracle) > wrote: > > This patch seris started out as part of the THP patch set, but it has > some nice effects along the way and it seems worth splitting it out and > submitting separately. > > Currently find_get_entry() and find_lock_entry() return the page > corresponding to the requested index, but the first thing most callers do > is find the head page, which we just threw away. As part of auditing > all the callers, I found some misuses of the APIs and some plain > inefficiencies that I've fixed. > > The diffstat is unflattering, but I added more kernel-doc. > > Matthew Wilcox (Oracle) (8): > mm: Factor find_get_swap_page out of mincore_page > mm: Use find_get_swap_page in memcontrol > mm: Optimise madvise WILLNEED > proc: Optimise smaps for shmem entries > i915: Use find_lock_page instead of find_lock_entry > mm: Convert find_get_entry to return the head page > mm: Return head page from find_lock_entry > mm: Hoist find_subpage call further up in pagecache_get_page > > drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 4 +-- > fs/proc/task_mmu.c| 8 + > include/linux/pagemap.h | 16 +++-- > include/linux/swap.h | 7 > mm/filemap.c | 41 +++ > mm/madvise.c | 21 +++- > mm/memcontrol.c | 25 ++ > mm/mincore.c | 28 ++-- > mm/shmem.c| 15 + > mm/swap_state.c | 31 + > 10 files changed, 98 insertions(+), 98 deletions(-) For the series: Reviewed-by: William Kucharski ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/8] Convert the intel iommu driver to the dma-iommu api
Hi Logan/All,
I have added a check for the sg_dma_len == 0 :
"""
} __sgt_iter(struct scatterlist *sgl, bool dma) {
struct sgt_iter s = { .sgp = sgl };
+ if (sgl && sg_dma_len(sgl) == 0)
+ s.sgp = NULL;
if (s.sgp) {
.
"""
at location [1].
but it doens't fix the problem.
You're right though, this change does need to be made, this code
doesn't handle pages of sg_dma_len(sg) == 0 correctly
So my guess is that we have more bugs in other parts of the i915
driver (or there is a problem with my "sg_dma_len == 0" fix above).
I have been trying to spot where else the code might be buggy but I
haven't had any luck so far.
I'm doing a microconfernce (at LPC 2020) this wednesdays [1] on this
if you're interested in attending.
I'm hoping I can chat about it with a few people and find how can
reproduce and fix this issues. I don't have any more time I can give
to this unfortunately and it would be a shame for the work to go to
waste.
[0]
https://github.com/torvalds/linux/blob/d012a7190fc1fd72ed48911e77ca97ba4521bccd/drivers/gpu/drm/i915/i915_scatterlist.h#L28
[1] https://linuxplumbersconf.org/event/7/contributions/846/
On Fri, 29 May 2020 at 22:21, Logan Gunthorpe wrote:
>
>
>
> On 2020-05-29 3:11 p.m., Marek Szyprowski wrote:
> > Patches are pending:
> > https://lore.kernel.org/linux-iommu/20200513132114.6046-1-m.szyprow...@samsung.com/T/
>
> Cool, nice! Though, I still don't think that fixes the issue in
> i915_scatterlist.h given it still ignores sg_dma_len() and strictly
> relies on sg_next()/sg_is_last() to stop iterating -- and I suspect this
> is the bug that got in Tom's way.
>
> >> However, as Robin pointed out, there are other ugly tricks like stopping
> >> iterating through the SGL when sg_dma_len() is zero. For example, the
> >> AMD driver appears to use drm_prime_sg_to_page_addr_arrays() which does
> >> this trick and thus likely isn't buggy (otherwise, I'd expect someone to
> >> have complained by now seeing AMD has already switched to IOMMU-DMA.
> >
> > I'm not sure that this is a trick. Stopping at zero sg_dma_len() was
> > somewhere documented.
>
> Well whatever you want to call it, it is ugly to have some drivers doing
> one thing with the returned value and others assuming there's an extra
> zero at the end. It just causes confusion for people reading/copying the
> code. It would be better if they are all consistent. However, I concede
> stopping at zero should not be broken, presently.
>
> Logan
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/gt: Show engine properties in the pretty printer
When debugging the engine state, include the user properties that may,
or may not, have been adjusted by the user/test.
For example,
vecs0
...
Properties:
heartbeat_interval_ms: 2500 [default 2500]
max_busywait_duration_ns: 8000 [default 8000]
preempt_timeout_ms: 640 [default 640]
stop_timeout_ms: 100 [default 100]
timeslice_duration_ms: 1 [default 1]
Suggested-by: Joonas Lahtinen
Signed-off-by: Chris Wilson
Cc: Joonas Lahtinen
Cc: Mika Kuoppala
---
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 36 +++
1 file changed, 36 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index ea4ba2afe9f9..bb9e3bb0c0c2 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -1599,6 +1599,41 @@ static unsigned long list_count(struct list_head *list)
return count;
}
+static unsigned long read_ul(void *p, size_t x)
+{
+ return *(unsigned long *)(p + x);
+}
+
+static void print_properties(struct intel_engine_cs *engine,
+struct drm_printer *m)
+{
+ static const struct pmap {
+ size_t offset;
+ const char *name;
+ } props[] = {
+#define P(x) { \
+ .offset = offsetof(typeof(engine->props), x), \
+ .name = #x \
+}
+ P(heartbeat_interval_ms),
+ P(max_busywait_duration_ns),
+ P(preempt_timeout_ms),
+ P(stop_timeout_ms),
+ P(timeslice_duration_ms),
+
+ {},
+#undef P
+ };
+ const struct pmap *p;
+
+ drm_printf(m, "Properties:\n");
+ for (p = props; p->name; p++)
+ drm_printf(m, "\t\t%s: %lu [default %lu]\n",
+ p->name,
+ read_ul(&engine->props, p->offset),
+ read_ul(&engine->defaults, p->offset));
+}
+
void intel_engine_dump(struct intel_engine_cs *engine,
struct drm_printer *m,
const char *header, ...)
@@ -1641,6 +1676,7 @@ void intel_engine_dump(struct intel_engine_cs *engine,
drm_printf(m, "\tReset count: %d (global %d)\n",
i915_reset_engine_count(error, engine),
i915_reset_count(error));
+ print_properties(engine, m);
drm_printf(m, "\tRequests:\n");
--
2.20.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [1/5] Fixing Possible Null Pointer Dereference
== Series Details == Series: series starting with [1/5] Fixing Possible Null Pointer Dereference URL : https://patchwork.freedesktop.org/series/80939/ State : failure == Summary == CI Bug Log - changes from CI_DRM_8918_full -> Patchwork_18392_full Summary --- **FAILURE** Serious unknown changes coming with Patchwork_18392_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_18392_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_18392_full: ### IGT changes ### Possible regressions * igt@gem_ctx_isolation@preservation-s3@bcs0: - shard-skl: [PASS][1] -> [INCOMPLETE][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-skl8/igt@gem_ctx_isolation@preservation...@bcs0.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18392/shard-skl3/igt@gem_ctx_isolation@preservation...@bcs0.html Known issues Here are the changes found in Patchwork_18392_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_exec_whisper@basic-forked: - shard-glk: [PASS][3] -> [DMESG-WARN][4] ([i915#118] / [i915#95]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-glk6/igt@gem_exec_whis...@basic-forked.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18392/shard-glk9/igt@gem_exec_whis...@basic-forked.html * igt@i915_pm_rpm@system-suspend: - shard-skl: [PASS][5] -> [INCOMPLETE][6] ([i915#151]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-skl7/igt@i915_pm_...@system-suspend.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18392/shard-skl4/igt@i915_pm_...@system-suspend.html * igt@i915_selftest@mock@contexts: - shard-apl: [PASS][7] -> [INCOMPLETE][8] ([i915#1635] / [i915#2278]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-apl8/igt@i915_selftest@m...@contexts.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18392/shard-apl1/igt@i915_selftest@m...@contexts.html * igt@kms_color@pipe-b-ctm-negative: - shard-skl: [PASS][9] -> [DMESG-WARN][10] ([i915#1982]) +12 similar issues [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-skl1/igt@kms_co...@pipe-b-ctm-negative.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18392/shard-skl5/igt@kms_co...@pipe-b-ctm-negative.html * igt@kms_flip@flip-vs-expired-vblank-interruptible@c-dp1: - shard-apl: [PASS][11] -> [FAIL][12] ([i915#1635] / [i915#79]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-apl8/igt@kms_flip@flip-vs-expired-vblank-interrupti...@c-dp1.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18392/shard-apl8/igt@kms_flip@flip-vs-expired-vblank-interrupti...@c-dp1.html * igt@kms_frontbuffer_tracking@fbc-suspend: - shard-kbl: [PASS][13] -> [DMESG-WARN][14] ([i915#180]) +5 similar issues [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-kbl7/igt@kms_frontbuffer_track...@fbc-suspend.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18392/shard-kbl4/igt@kms_frontbuffer_track...@fbc-suspend.html * igt@kms_hdr@bpc-switch-dpms: - shard-skl: [PASS][15] -> [FAIL][16] ([i915#1188]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-skl9/igt@kms_...@bpc-switch-dpms.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18392/shard-skl3/igt@kms_...@bpc-switch-dpms.html * igt@kms_psr@psr2_sprite_blt: - shard-iclb: [PASS][17] -> [SKIP][18] ([fdo#109441]) +1 similar issue [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-iclb2/igt@kms_psr@psr2_sprite_blt.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18392/shard-iclb6/igt@kms_psr@psr2_sprite_blt.html * igt@perf_pmu@module-unload: - shard-tglb: [PASS][19] -> [DMESG-WARN][20] ([i915#1982]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-tglb3/igt@perf_...@module-unload.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18392/shard-tglb2/igt@perf_...@module-unload.html * igt@prime_busy@hang@bcs0: - shard-hsw: [PASS][21] -> [FAIL][22] ([i915#2258]) +4 similar issues [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-hsw8/igt@prime_busy@h...@bcs0.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18392/shard-hsw4/igt@prime_busy@h...@bcs0.html Possible fixes * igt@gem_exec_create@forked: - shard-glk: [DMESG-WARN][23] ([i915#118] / [i915#95]) -> [PASS][24] [23]:
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gt: Show engine properties in the pretty printer
== Series Details == Series: drm/i915/gt: Show engine properties in the pretty printer URL : https://patchwork.freedesktop.org/series/80947/ State : warning == Summary == $ dim checkpatch origin/drm-tip 6ef144300979 drm/i915/gt: Show engine properties in the pretty printer -:48: CHECK:LINE_SPACING: Please use a blank line after function/struct/union/enum declarations #48: FILE: drivers/gpu/drm/i915/gt/intel_engine_cs.c:1618: +} + P(heartbeat_interval_ms), total: 0 errors, 0 warnings, 1 checks, 48 lines checked ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/gt: Show engine properties in the pretty printer
== Series Details == Series: drm/i915/gt: Show engine properties in the pretty printer URL : https://patchwork.freedesktop.org/series/80947/ State : success == Summary == CI Bug Log - changes from CI_DRM_8918 -> Patchwork_18394 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18394/index.html Known issues Here are the changes found in Patchwork_18394 that come from known issues: ### IGT changes ### Issues hit * igt@i915_module_load@reload: - fi-bsw-kefka: [PASS][1] -> [DMESG-WARN][2] ([i915#1982]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/fi-bsw-kefka/igt@i915_module_l...@reload.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18394/fi-bsw-kefka/igt@i915_module_l...@reload.html * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic: - fi-byt-j1900: [PASS][3] -> [DMESG-WARN][4] ([i915#1982]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/fi-byt-j1900/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18394/fi-byt-j1900/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html Possible fixes * igt@i915_pm_rpm@basic-pci-d3-state: - fi-byt-j1900: [DMESG-WARN][5] ([i915#1982]) -> [PASS][6] [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/fi-byt-j1900/igt@i915_pm_...@basic-pci-d3-state.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18394/fi-byt-j1900/igt@i915_pm_...@basic-pci-d3-state.html - fi-bsw-n3050: [DMESG-WARN][7] ([i915#1982]) -> [PASS][8] [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/fi-bsw-n3050/igt@i915_pm_...@basic-pci-d3-state.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18394/fi-bsw-n3050/igt@i915_pm_...@basic-pci-d3-state.html - fi-bsw-kefka: [DMESG-WARN][9] ([i915#1982]) -> [PASS][10] [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/fi-bsw-kefka/igt@i915_pm_...@basic-pci-d3-state.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18394/fi-bsw-kefka/igt@i915_pm_...@basic-pci-d3-state.html * igt@kms_flip@basic-flip-vs-wf_vblank@c-hdmi-a2: - fi-skl-guc: [DMESG-WARN][11] ([i915#2203]) -> [PASS][12] [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/fi-skl-guc/igt@kms_flip@basic-flip-vs-wf_vbl...@c-hdmi-a2.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18394/fi-skl-guc/igt@kms_flip@basic-flip-vs-wf_vbl...@c-hdmi-a2.html Warnings * igt@i915_pm_rpm@module-reload: - fi-kbl-x1275: [DMESG-FAIL][13] ([i915#62] / [i915#95]) -> [DMESG-FAIL][14] ([i915#62]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/fi-kbl-x1275/igt@i915_pm_...@module-reload.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18394/fi-kbl-x1275/igt@i915_pm_...@module-reload.html * igt@kms_flip@basic-flip-vs-modeset@a-dp1: - fi-kbl-x1275: [DMESG-WARN][15] ([i915#62] / [i915#92]) -> [DMESG-WARN][16] ([i915#62] / [i915#92] / [i915#95]) +6 similar issues [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-mode...@a-dp1.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18394/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-mode...@a-dp1.html * igt@prime_vgem@basic-fence-flip: - fi-kbl-x1275: [DMESG-WARN][17] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][18] ([i915#62] / [i915#92]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/fi-kbl-x1275/igt@prime_v...@basic-fence-flip.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18394/fi-kbl-x1275/igt@prime_v...@basic-fence-flip.html [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982 [i915#2203]: https://gitlab.freedesktop.org/drm/intel/issues/2203 [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62 [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92 [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95 Participating hosts (37 -> 34) -- Missing(3): fi-byt-clapper fi-byt-squawks fi-bsw-cyan Build changes - * Linux: CI_DRM_8918 -> Patchwork_18394 CI-20190529: 20190529 CI_DRM_8918: 7296072da95d2d2550ef2d4836bc72663ee711cc @ git://anongit.freedesktop.org/gfx-ci/linux IGT_5770: f1d0c240ea2e631dfb9f493f37f8fb61cb2b1cf2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_18394: 6ef1443009794f8cc7679a91b813f7c9695339f0 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 6ef144300979 drm/i915/gt: Show engine properties in the pretty printer == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18394/index.html ___
[Intel-gfx] [PATCH i-g-t] i915/gem_exec_schedule: Set preempt_timeout_ms for fast hang tests
Reduce the preemption timeout to 150ms (from infinity for tgl! tsk,
tsk) so that the preemption hang tests run quicker.
Signed-off-by: Chris Wilson
---
tests/i915/gem_exec_schedule.c | 37 --
1 file changed, 26 insertions(+), 11 deletions(-)
diff --git a/tests/i915/gem_exec_schedule.c b/tests/i915/gem_exec_schedule.c
index 488d93511..e316cf4d7 100644
--- a/tests/i915/gem_exec_schedule.c
+++ b/tests/i915/gem_exec_schedule.c
@@ -1235,9 +1235,18 @@ static void promotion(int fd, unsigned ring)
igt_assert_eq_u32(result_read, ctx[NOISE]);
}
+static bool set_preempt_timeout(int i915,
+ const struct intel_execution_engine2 *e,
+ int timeout_ms)
+{
+ return gem_engine_property_printf(i915, e->name,
+ "preempt_timeout_ms",
+ "%d", timeout_ms) > 0;
+}
+
#define NEW_CTX (0x1 << 0)
#define HANG_LP (0x1 << 1)
-static void preempt(int fd, unsigned ring, unsigned flags)
+static void preempt(int fd, const struct intel_execution_engine2 *e, unsigned
flags)
{
uint32_t result = gem_create(fd, 4096);
uint32_t result_read;
@@ -1245,6 +1254,9 @@ static void preempt(int fd, unsigned ring, unsigned flags)
uint32_t ctx[2];
igt_hang_t hang;
+ /* Set a fast timeout to speed the test up (if available) */
+ set_preempt_timeout(fd, e, 150);
+
ctx[LO] = gem_context_clone_with_engines(fd, 0);
gem_context_set_priority(fd, ctx[LO], MIN_PRIO);
@@ -1252,7 +1264,7 @@ static void preempt(int fd, unsigned ring, unsigned flags)
gem_context_set_priority(fd, ctx[HI], MAX_PRIO);
if (flags & HANG_LP)
- hang = igt_hang_ctx(fd, ctx[LO], ring, 0);
+ hang = igt_hang_ctx(fd, ctx[LO], e->flags, 0);
for (int n = 0; n < ARRAY_SIZE(spin); n++) {
if (flags & NEW_CTX) {
@@ -1262,10 +1274,10 @@ static void preempt(int fd, unsigned ring, unsigned
flags)
}
spin[n] = __igt_spin_new(fd,
.ctx = ctx[LO],
-.engine = ring);
+.engine = e->flags);
igt_debug("spin[%d].handle=%d\n", n, spin[n]->handle);
- store_dword(fd, ctx[HI], ring, result, 0, n + 1,
I915_GEM_DOMAIN_RENDER);
+ store_dword(fd, ctx[HI], e->flags, result, 0, n + 1,
I915_GEM_DOMAIN_RENDER);
result_read = __sync_read_u32(fd, result, 0);
igt_assert_eq_u32(result_read, n + 1);
@@ -1601,12 +1613,15 @@ static void preempt_self(int fd, unsigned ring)
gem_close(fd, result);
}
-static void preemptive_hang(int fd, unsigned ring)
+static void preemptive_hang(int fd, const struct intel_execution_engine2 *e)
{
igt_spin_t *spin[MAX_ELSP_QLEN];
igt_hang_t hang;
uint32_t ctx[2];
+ /* Set a fast timeout to speed the test up (if available) */
+ set_preempt_timeout(fd, e, 150);
+
ctx[HI] = gem_context_clone_with_engines(fd, 0);
gem_context_set_priority(fd, ctx[HI], MAX_PRIO);
@@ -1616,12 +1631,12 @@ static void preemptive_hang(int fd, unsigned ring)
spin[n] = __igt_spin_new(fd,
.ctx = ctx[LO],
-.engine = ring);
+.engine = e->flags);
gem_context_destroy(fd, ctx[LO]);
}
- hang = igt_hang_ctx(fd, ctx[HI], ring, 0);
+ hang = igt_hang_ctx(fd, ctx[HI], e->flags, 0);
igt_post_hang_ring(fd, hang);
for (int n = 0; n < ARRAY_SIZE(spin); n++) {
@@ -2603,10 +2618,10 @@ igt_main
}
test_each_engine_store("preempt", fd, e)
- preempt(fd, e->flags, 0);
+ preempt(fd, e, 0);
test_each_engine_store("preempt-contexts", fd, e)
- preempt(fd, e->flags, NEW_CTX);
+ preempt(fd, e, NEW_CTX);
test_each_engine_store("preempt-self", fd, e)
preempt_self(fd, e->flags);
@@ -2640,10 +2655,10 @@ igt_main
}
test_each_engine_store("preempt-hang", fd, e)
- preempt(fd, e->flags, NEW_CTX |
HANG_LP);
+ preempt(fd, e, NEW_CTX | HANG_LP);
test_each_engine_store("preemptive-hang", fd, e)
- preemptive_hang(fd, e->flags);
+ preemptive_hang(fd, e);
igt_fixture {
igt_disallow_hang(fd,
Re: [Intel-gfx] [RFC 13/20] drm/i915/dp: Extract drm_dp_downstream_read_info()
On Fri, Aug 21, 2020 at 01:43:39PM -0400, Lyude Paul wrote: > [...] > > The wording is a bit unclear, but as I understand the Standard only > > calls for the above: > > > > """ > > A DP upstream device shall read the capability from DPCD Addresses 00080h > > through 00083h. A DP Branch device with multiple DFPs shall report the > > detailed > > capability information of the lowest DFP number to which a downstream device > > is connected, consistent with the DisplayID or legacy EDID access routing > > policy > > of an SST-only DP Branch device as described in Section 2.1.4.1. > > """ > > So-I saw this too, but notice the use of the language "A /DP Branch/ device > with > multiple DFPs shall report the detailed…". This makes me think it's implying > that this is a requirement for MSTBs and not SST sinks, just a guess. Not sure either. The above could also refer to an SST branch device with multiple DFPs (for instance a DP Replicator branch device). --Imre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for acpi/pwm/i915: Convert pwm-crc and i915 driver's PWM code to use the atomic PWM API
== Series Details == Series: acpi/pwm/i915: Convert pwm-crc and i915 driver's PWM code to use the atomic PWM API URL : https://patchwork.freedesktop.org/series/80943/ State : success == Summary == CI Bug Log - changes from CI_DRM_8918_full -> Patchwork_18393_full Summary --- **SUCCESS** No regressions found. Known issues Here are the changes found in Patchwork_18393_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_ctx_isolation@preservation-s3@rcs0: - shard-kbl: [PASS][1] -> [DMESG-WARN][2] ([i915#180]) +4 similar issues [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-kbl7/igt@gem_ctx_isolation@preservation...@rcs0.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18393/shard-kbl7/igt@gem_ctx_isolation@preservation...@rcs0.html * igt@gem_exec_balancer@bonded-early: - shard-kbl: [PASS][3] -> [FAIL][4] ([i915#2079]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-kbl2/igt@gem_exec_balan...@bonded-early.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18393/shard-kbl2/igt@gem_exec_balan...@bonded-early.html * igt@gem_exec_whisper@basic-forked-all: - shard-glk: [PASS][5] -> [DMESG-WARN][6] ([i915#118] / [i915#95]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-glk6/igt@gem_exec_whis...@basic-forked-all.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18393/shard-glk6/igt@gem_exec_whis...@basic-forked-all.html * igt@gem_partial_pwrite_pread@reads-uncached: - shard-apl: [PASS][7] -> [FAIL][8] ([i915#1635]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-apl3/igt@gem_partial_pwrite_pr...@reads-uncached.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18393/shard-apl2/igt@gem_partial_pwrite_pr...@reads-uncached.html * igt@kms_big_fb@linear-64bpp-rotate-0: - shard-glk: [PASS][9] -> [DMESG-FAIL][10] ([i915#118] / [i915#95]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-glk1/igt@kms_big...@linear-64bpp-rotate-0.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18393/shard-glk8/igt@kms_big...@linear-64bpp-rotate-0.html * igt@kms_big_fb@x-tiled-32bpp-rotate-180: - shard-skl: [PASS][11] -> [DMESG-WARN][12] ([i915#1982]) +16 similar issues [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-skl6/igt@kms_big...@x-tiled-32bpp-rotate-180.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18393/shard-skl5/igt@kms_big...@x-tiled-32bpp-rotate-180.html * igt@kms_draw_crc@draw-method-rgb565-mmap-gtt-untiled: - shard-apl: [PASS][13] -> [DMESG-WARN][14] ([i915#1635] / [i915#1982]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-apl3/igt@kms_draw_...@draw-method-rgb565-mmap-gtt-untiled.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18393/shard-apl4/igt@kms_draw_...@draw-method-rgb565-mmap-gtt-untiled.html * igt@kms_flip@flip-vs-suspend@c-edp1: - shard-skl: [PASS][15] -> [INCOMPLETE][16] ([i915#198]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-skl10/igt@kms_flip@flip-vs-susp...@c-edp1.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18393/shard-skl4/igt@kms_flip@flip-vs-susp...@c-edp1.html * igt@kms_frontbuffer_tracking@fbc-stridechange: - shard-tglb: [PASS][17] -> [DMESG-WARN][18] ([i915#1982]) +1 similar issue [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-tglb2/igt@kms_frontbuffer_track...@fbc-stridechange.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18393/shard-tglb1/igt@kms_frontbuffer_track...@fbc-stridechange.html * igt@kms_hdr@bpc-switch-dpms: - shard-skl: [PASS][19] -> [FAIL][20] ([i915#1188]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-skl9/igt@kms_...@bpc-switch-dpms.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18393/shard-skl8/igt@kms_...@bpc-switch-dpms.html * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: - shard-glk: [PASS][21] -> [DMESG-WARN][22] ([i915#1982]) +1 similar issue [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-glk6/igt@kms_pipe_crc_ba...@suspend-read-crc-pipe-c.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18393/shard-glk8/igt@kms_pipe_crc_ba...@suspend-read-crc-pipe-c.html * igt@kms_psr@psr2_sprite_blt: - shard-iclb: [PASS][23] -> [SKIP][24] ([fdo#109441]) +1 similar issue [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-iclb2/igt@kms_psr@psr2_sprite_blt.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18393/shard-iclb7/igt@kms_psr@psr2_sprite_blt.html * igt@perf@polling-parameterized: - shard-tglb: [PASS][25] ->
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Compute has_drrs after compute has_psr
On Mon, 2020-08-24 at 16:24 +0530, Anshuman Gupta wrote:
> On 2020-08-20 at 10:23:52 -0700, José Roberto de Souza wrote:
> > DRRS and PSR can't be enable together, so giving preference to PSR
> > as it allows more power-savings by complete shutting down display,
> > so to guarantee this, it should compute DRRS state after compute PSR.
> >
> > Cc: Srinivas K <
> > srinivas...@intel.com
> > >
> > Cc: Hariom Pandey <
> > hariom.pan...@intel.com
> > >
> > Signed-off-by: José Roberto de Souza <
> > jose.so...@intel.com
> > >
> > ---
> > drivers/gpu/drm/i915/display/intel_dp.c | 52 +++--
> > 1 file changed, 31 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 79c27f91f42c..3bf50b1ae983 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -2575,6 +2575,34 @@ intel_dp_compute_hdr_metadata_infoframe_sdp(struct
> > intel_dp *intel_dp,
> > intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_GAMUT_METADATA);
> > }
> >
> > +static void
> > +intel_dp_drrs_compute_config(struct intel_dp *intel_dp,
> > +struct intel_crtc_state *pipe_config,
> > +int output_bpp, bool constant_n)
> > +{
> > + struct intel_connector *intel_connector = intel_dp->attached_connector;
> > + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > +
> > + /*
> > +* DRRS and PSR can't be enable together, so giving preference to PSR
> > +* as it allows more power-savings by complete shutting down display,
> > +* so to guarantee this, intel_dp_drrs_compute_config() must be called
> > +* after intel_psr_compute_config().
> > +*/
> > + if (pipe_config->has_psr)
> > + return;
>
> In cases when psr is supported but is not enabled due to any psr error, drrs
> can be useful.
> AFAIU has_psr is false in cases psr is disabled due any PSR error, this
> should be fine for such case
sink_not_reliable is checked in intel_psr_compute_config() so after a PSR error
in the next time fastset(in the next patch) DRRS will be enabled.
> > +
> > + if (!intel_connector->panel.downclock_mode ||
> > + dev_priv->drrs.type != SEAMLESS_DRRS_SUPPORT)
> > + return;
> > +
> > + pipe_config->has_drrs = true;
> > + intel_link_compute_m_n(output_bpp, pipe_config->lane_count,
> > + intel_connector->panel.downclock_mode->clock,
> > + pipe_config->port_clock, &pipe_config->dp_m2_n2,
> > + constant_n, pipe_config->fec_enable);
> > +}
> > +
> > int
> > intel_dp_compute_config(struct intel_encoder *encoder,
> > struct intel_crtc_state *pipe_config,
> > @@ -2605,7 +2633,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> > if (ret)
> > return ret;
> >
> > - pipe_config->has_drrs = false;
>
> May be u can retain it for better readability.
The whole struct is zeroed, other features are not set false if not supported.
> > if (!intel_dp_port_has_audio(dev_priv, port))
> > pipe_config->has_audio = false;
> > else if (intel_conn_state->force_audio == HDMI_AUDIO_AUTO)
> > @@ -2657,21 +2684,12 @@ intel_dp_compute_config(struct intel_encoder
> > *encoder,
> >&pipe_config->dp_m_n,
> >constant_n, pipe_config->fec_enable);
> >
> > - if (intel_connector->panel.downclock_mode != NULL &&
> > - dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
> > - pipe_config->has_drrs = true;
> > - intel_link_compute_m_n(output_bpp,
> > - pipe_config->lane_count,
> > -
> > intel_connector->panel.downclock_mode->clock,
> > - pipe_config->port_clock,
> > - &pipe_config->dp_m2_n2,
> > - constant_n,
> > pipe_config->fec_enable);
> > - }
> > -
> > if (!HAS_DDI(dev_priv))
> > intel_dp_set_clock(encoder, pipe_config);
> >
> > intel_psr_compute_config(intel_dp, pipe_config);
> > + intel_dp_drrs_compute_config(intel_dp, pipe_config, output_bpp,
> > +constant_n);
> > intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state);
> > intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp, pipe_config,
> > conn_state);
> >
> > @@ -7730,16 +7748,8 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp,
> > {
> > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> >
> > - if (!crtc_state->has_drrs) {
> > - drm_dbg_kms(&dev_priv->drm, "Panel doesn't support DRRS\n");
>
> IMHO this print is useful.
Okay but will change the message to DRRS enabled, we already know that the
panel su
Re: [Intel-gfx] [PATCH 2/2] drm/i915/drrs: Disable DRRS when needed in fastsets
On Mon, 2020-08-24 at 14:21 +0530, Anshuman Gupta wrote:
> On 2020-08-20 at 22:53:53 +0530, José Roberto de Souza wrote:
> > Changes in the configuration could cause PSR to be compatible and
> > enabled so driver must also be able to disable DRRS when doing
> > fastsets.
> >
> > Closes:
> > https://gitlab.freedesktop.org/drm/intel/-/issues/209
> >
> > Closes:
> > https://gitlab.freedesktop.org/drm/intel/-/issues/173
> >
> > Closes:
> > https://gitlab.freedesktop.org/drm/intel/-/issues/209
> >
> > Cc: Srinivas K <
> > srinivas...@intel.com
> > >
> > Cc: Hariom Pandey <
> > hariom.pan...@intel.com
> > >
> > Signed-off-by: José Roberto de Souza <
> > jose.so...@intel.com
> > >
>
> overall patch looks goood to me,
> > ---
> > drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
> > drivers/gpu/drm/i915/display/intel_dp.c | 84 +++-
> > drivers/gpu/drm/i915/display/intel_dp.h | 2 +
> > 3 files changed, 71 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index de5b216561d8..ff05a852417c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -4012,7 +4012,7 @@ static void intel_ddi_update_pipe_dp(struct
> > intel_atomic_state *state,
> >
> > intel_psr_update(intel_dp, crtc_state, conn_state);
> > intel_dp_set_infoframes(encoder, true, crtc_state, conn_state);
> > - intel_edp_drrs_enable(intel_dp, crtc_state);
> > + intel_edp_drrs_update(intel_dp, crtc_state);
> >
> > intel_panel_update_backlight(state, encoder, crtc_state, conn_state);
> > }
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 3bf50b1ae983..de2c9851395d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -2576,9 +2576,9 @@ intel_dp_compute_hdr_metadata_infoframe_sdp(struct
> > intel_dp *intel_dp,
> > }
> >
> > static void
> > -intel_dp_drrs_compute_config(struct intel_dp *intel_dp,
> > -struct intel_crtc_state *pipe_config,
> > -int output_bpp, bool constant_n)
> > +intel_dp_compute_drrs(struct intel_dp *intel_dp,
> > + struct intel_crtc_state *pipe_config,
> > + int output_bpp, bool constant_n)
>
> intel_dp_drrs_compute_config looks a better name which u have introduced in
> patch 1 of this series.
> any reason to change the function name in second patch ?
Thanks for catching this, changing back to the name in patch 1.
> > {
> > struct intel_connector *intel_connector = intel_dp->attached_connector;
> > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > @@ -2586,8 +2586,8 @@ intel_dp_drrs_compute_config(struct intel_dp
> > *intel_dp,
> > /*
> > * DRRS and PSR can't be enable together, so giving preference to PSR
> > * as it allows more power-savings by complete shutting down display,
> > -* so to guarantee this, intel_dp_drrs_compute_config() must be called
> > -* after intel_psr_compute_config().
> > +* so to guarantee this, intel_dp_compute_drrs() must be called after
> > +* intel_psr_compute_config().
> > */
> > if (pipe_config->has_psr)
> > return;
> > @@ -2688,8 +2688,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> > intel_dp_set_clock(encoder, pipe_config);
> >
> > intel_psr_compute_config(intel_dp, pipe_config);
> > - intel_dp_drrs_compute_config(intel_dp, pipe_config, output_bpp,
> > -constant_n);
> > + intel_dp_compute_drrs(intel_dp, pipe_config, output_bpp, constant_n);
> > intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state);
> > intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp, pipe_config,
> > conn_state);
> >
> > @@ -7736,6 +7735,15 @@ static void intel_dp_set_drrs_state(struct
> > drm_i915_private *dev_priv,
> > refresh_rate);
> > }
> >
> > +static void
> > +intel_edp_drrs_enable_locked(struct intel_dp *intel_dp)
> > +{
> > + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > +
> > + dev_priv->drrs.busy_frontbuffer_bits = 0;
> > + dev_priv->drrs.dp = intel_dp;
> > +}
> > +
> > /**
> > * intel_edp_drrs_enable - init drrs struct if supported
> > * @intel_dp: DP struct
> > @@ -7752,19 +7760,34 @@ void intel_edp_drrs_enable(struct intel_dp
> > *intel_dp,
> > return;
> >
> > mutex_lock(&dev_priv->drrs.mutex);
> > +
> > if (dev_priv->drrs.dp) {
> > - drm_dbg_kms(&dev_priv->drm, "DRRS already enabled\n");
> > + drm_warn(&dev_priv->drm, "DRRS already enabled\n");
> > goto unlock;
> > }
> >
> > - dev_priv->drrs.busy_frontbuffer_bits = 0;
> > -
> > - dev_priv->drrs.dp = intel_dp;
> > + intel_edp_drrs_enable_locked(intel_dp);
> >
> > unlock:
> > mutex_unl
[Intel-gfx] [PATCH v2 2/3] drm/i915/display: Disable DRRS when needed in fastsets
Changes in the configuration could cause PSR to be compatible and
enabled so driver must also be able to disable DRRS when doing
fastsets.
v2: Fixed name of DRRS compute function (Anshuman)
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/209
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/173
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/209
Cc: Srinivas K
Cc: Hariom Pandey
Cc: Anshuman Gupta
Signed-off-by: José Roberto de Souza
---
drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
drivers/gpu/drm/i915/display/intel_dp.c | 71 +---
drivers/gpu/drm/i915/display/intel_dp.h | 2 +
3 files changed, 65 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
b/drivers/gpu/drm/i915/display/intel_ddi.c
index de5b216561d8..ff05a852417c 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -4012,7 +4012,7 @@ static void intel_ddi_update_pipe_dp(struct
intel_atomic_state *state,
intel_psr_update(intel_dp, crtc_state, conn_state);
intel_dp_set_infoframes(encoder, true, crtc_state, conn_state);
- intel_edp_drrs_enable(intel_dp, crtc_state);
+ intel_edp_drrs_update(intel_dp, crtc_state);
intel_panel_update_backlight(state, encoder, crtc_state, conn_state);
}
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index a08d03c61b02..c57ac83bf563 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -7736,6 +7736,15 @@ static void intel_dp_set_drrs_state(struct
drm_i915_private *dev_priv,
refresh_rate);
}
+static void
+intel_edp_drrs_enable_locked(struct intel_dp *intel_dp)
+{
+ struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+
+ dev_priv->drrs.busy_frontbuffer_bits = 0;
+ dev_priv->drrs.dp = intel_dp;
+}
+
/**
* intel_edp_drrs_enable - init drrs struct if supported
* @intel_dp: DP struct
@@ -7754,19 +7763,34 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp,
drm_dbg_kms(&dev_priv->drm, "Enabling DRRS\n");
mutex_lock(&dev_priv->drrs.mutex);
+
if (dev_priv->drrs.dp) {
- drm_dbg_kms(&dev_priv->drm, "DRRS already enabled\n");
+ drm_warn(&dev_priv->drm, "DRRS already enabled\n");
goto unlock;
}
- dev_priv->drrs.busy_frontbuffer_bits = 0;
-
- dev_priv->drrs.dp = intel_dp;
+ intel_edp_drrs_enable_locked(intel_dp);
unlock:
mutex_unlock(&dev_priv->drrs.mutex);
}
+static void
+intel_edp_drrs_disable_locked(struct intel_dp *intel_dp,
+ const struct intel_crtc_state *crtc_state)
+{
+ struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+
+ if (dev_priv->drrs.refresh_rate_type == DRRS_LOW_RR) {
+ int refresh;
+
+ refresh =
drm_mode_vrefresh(intel_dp->attached_connector->panel.fixed_mode);
+ intel_dp_set_drrs_state(dev_priv, crtc_state, refresh);
+ }
+
+ dev_priv->drrs.dp = NULL;
+}
+
/**
* intel_edp_drrs_disable - Disable DRRS
* @intel_dp: DP struct
@@ -7787,16 +7811,45 @@ void intel_edp_drrs_disable(struct intel_dp *intel_dp,
return;
}
- if (dev_priv->drrs.refresh_rate_type == DRRS_LOW_RR)
- intel_dp_set_drrs_state(dev_priv, old_crtc_state,
-
drm_mode_vrefresh(intel_dp->attached_connector->panel.fixed_mode));
-
- dev_priv->drrs.dp = NULL;
+ intel_edp_drrs_disable_locked(intel_dp, old_crtc_state);
mutex_unlock(&dev_priv->drrs.mutex);
cancel_delayed_work_sync(&dev_priv->drrs.work);
}
+/**
+ * intel_edp_drrs_update - Update DRRS state
+ * @intel_dp: Intel DP
+ * @crtc_state: new CRTC state
+ *
+ * This function will update DRRS states, disabling or enabling DRRS when
+ * executing fastsets. For full modeset, intel_edp_drrs_disable() and
+ * intel_edp_drrs_enable() should be called instead.
+ */
+void
+intel_edp_drrs_update(struct intel_dp *intel_dp,
+ const struct intel_crtc_state *crtc_state)
+{
+ struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+
+ if (dev_priv->drrs.type != SEAMLESS_DRRS_SUPPORT)
+ return;
+
+ mutex_lock(&dev_priv->drrs.mutex);
+
+ /* New state matches current one? */
+ if (crtc_state->has_drrs == !!dev_priv->drrs.dp)
+ goto unlock;
+
+ if (crtc_state->has_drrs)
+ intel_edp_drrs_enable_locked(intel_dp);
+ else
+ intel_edp_drrs_disable_locked(intel_dp, crtc_state);
+
+unlock:
+ mutex_unlock(&dev_priv->drrs.mutex);
+}
+
static void intel_edp_drrs_downclock_work(struct work_struct *work)
{
struct drm_i915_private *dev_priv =
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h
b/drivers/gpu/drm/i915/display/intel_dp.h
index b901ab850c
[Intel-gfx] [PATCH v2 1/3] drm/i915/display: Compute has_drrs after compute has_psr
DRRS and PSR can't be enable together, so giving preference to PSR
as it allows more power-savings by complete shutting down display,
so to guarantee this, it should compute DRRS state after compute PSR.
Cc: Srinivas K
Cc: Hariom Pandey
Reviewed-by: Anshuman Gupta
Signed-off-by: José Roberto de Souza
---
drivers/gpu/drm/i915/display/intel_dp.c | 52 +++--
1 file changed, 32 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 79c27f91f42c..a08d03c61b02 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2575,6 +2575,34 @@ intel_dp_compute_hdr_metadata_infoframe_sdp(struct
intel_dp *intel_dp,
intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_GAMUT_METADATA);
}
+static void
+intel_dp_drrs_compute_config(struct intel_dp *intel_dp,
+struct intel_crtc_state *pipe_config,
+int output_bpp, bool constant_n)
+{
+ struct intel_connector *intel_connector = intel_dp->attached_connector;
+ struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+
+ /*
+* DRRS and PSR can't be enable together, so giving preference to PSR
+* as it allows more power-savings by complete shutting down display,
+* so to guarantee this, intel_dp_drrs_compute_config() must be called
+* after intel_psr_compute_config().
+*/
+ if (pipe_config->has_psr)
+ return;
+
+ if (!intel_connector->panel.downclock_mode ||
+ dev_priv->drrs.type != SEAMLESS_DRRS_SUPPORT)
+ return;
+
+ pipe_config->has_drrs = true;
+ intel_link_compute_m_n(output_bpp, pipe_config->lane_count,
+ intel_connector->panel.downclock_mode->clock,
+ pipe_config->port_clock, &pipe_config->dp_m2_n2,
+ constant_n, pipe_config->fec_enable);
+}
+
int
intel_dp_compute_config(struct intel_encoder *encoder,
struct intel_crtc_state *pipe_config,
@@ -2605,7 +2633,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
if (ret)
return ret;
- pipe_config->has_drrs = false;
if (!intel_dp_port_has_audio(dev_priv, port))
pipe_config->has_audio = false;
else if (intel_conn_state->force_audio == HDMI_AUDIO_AUTO)
@@ -2657,21 +2684,12 @@ intel_dp_compute_config(struct intel_encoder *encoder,
&pipe_config->dp_m_n,
constant_n, pipe_config->fec_enable);
- if (intel_connector->panel.downclock_mode != NULL &&
- dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
- pipe_config->has_drrs = true;
- intel_link_compute_m_n(output_bpp,
- pipe_config->lane_count,
-
intel_connector->panel.downclock_mode->clock,
- pipe_config->port_clock,
- &pipe_config->dp_m2_n2,
- constant_n,
pipe_config->fec_enable);
- }
-
if (!HAS_DDI(dev_priv))
intel_dp_set_clock(encoder, pipe_config);
intel_psr_compute_config(intel_dp, pipe_config);
+ intel_dp_drrs_compute_config(intel_dp, pipe_config, output_bpp,
+constant_n);
intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state);
intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp, pipe_config,
conn_state);
@@ -7730,16 +7748,10 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp,
{
struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
- if (!crtc_state->has_drrs) {
- drm_dbg_kms(&dev_priv->drm, "Panel doesn't support DRRS\n");
+ if (!crtc_state->has_drrs)
return;
- }
- if (dev_priv->psr.enabled) {
- drm_dbg_kms(&dev_priv->drm,
- "PSR enabled. Not enabling DRRS.\n");
- return;
- }
+ drm_dbg_kms(&dev_priv->drm, "Enabling DRRS\n");
mutex_lock(&dev_priv->drrs.mutex);
if (dev_priv->drrs.dp) {
--
2.28.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 3/3] drm/i915/display: Fix DRRS debugfs
Supported and enabled are different things so printing both.
Cc: Anshuman Gupta
Cc: Srinivas K
Cc: Hariom Pandey
Signed-off-by: José Roberto de Souza
---
drivers/gpu/drm/i915/display/intel_display_debugfs.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index f549381048b3..4b4cabf34d24 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -1069,10 +1069,18 @@ static void drrs_status_per_crtc(struct seq_file *m,
drm_connector_list_iter_begin(dev, &conn_iter);
drm_for_each_connector_iter(connector, &conn_iter) {
+ bool supported = false;
+
if (connector->state->crtc != &intel_crtc->base)
continue;
seq_printf(m, "%s:\n", connector->name);
+
+ if (connector->connector_type == DRM_MODE_CONNECTOR_eDP &&
+ dev_priv->vbt.drrs_type == SEAMLESS_DRRS_SUPPORT)
+ supported = true;
+
+ seq_printf(m, "\tDRRS Supported: %s\n", yesno(supported));
}
drm_connector_list_iter_end(&conn_iter);
@@ -1083,7 +1091,7 @@ static void drrs_status_per_crtc(struct seq_file *m,
mutex_lock(&drrs->mutex);
/* DRRS Supported */
- seq_puts(m, "\tDRRS Supported: Yes\n");
+ seq_puts(m, "\tDRRS Enabled: Yes\n");
/* disable_drrs() will make drrs->dp NULL */
if (!drrs->dp) {
@@ -1118,7 +1126,7 @@ static void drrs_status_per_crtc(struct seq_file *m,
mutex_unlock(&drrs->mutex);
} else {
/* DRRS not supported. Print the VBT parameter*/
- seq_puts(m, "\tDRRS Supported : No");
+ seq_puts(m, "\tDRRS Enabled : No");
}
seq_puts(m, "\n");
}
--
2.28.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Regression] "drm/i915: Implement display w/a #1143" breaks HDMI on ASUS GL552VW
On Mon, Aug 17, 2020 at 02:17:49PM +0800, Kai-Heng Feng wrote: > > > > On Aug 17, 2020, at 00:22, Runyan, Arthur J > > wrote: > > > > You'll need to read out the DDI_BUF_TRANS_* and DISPIO_CR_TX_BMU_CR0 > > registers at boot before i915 programs them and compare with what driver > > programs. > > Rodrigo can probably show you how. > > Right, I'll wait for a patch then :) To grab the BIOS reg values we just have to make sure the driver doesn't load. Eg. pass something like "modprobe.blacklist=i915,snd_hda_intel 3" to the kernel cmdline (+ whatever other magic ubuntu might require). Confirm with something like "lsmod | grep i915" to make sure the driver didn't sneak in despite our best efforts. Then we can dump the registers with intel_reg from igt-gpu-tools: intel_reg read --count 20 0x64E00 0x64E60 0x64EC0 0x64F20 0x64F80 intel_reg read 0x64000 0x64100 0x64200 0x64300 0x64400 0x6C00C The only somewhat suspicious thing I noticed is that we treat DISPIO_CR_TX_BMU_CR0:tx_blnclegdisbl as a bitmask (bit 23 -> DDI A, bit 24 -> DDI B, etc.) whereas the spec seems to be saying that we should just zero out all the bits of tx_blnclegdisbl when any DDI needs iboost. Art, is our interpretation of the bits correct or just a fairy tale? > > Kai-Heng > > > > > -Original Message- > > From: Kai-Heng Feng > > Sent: Thursday, August 13, 2020 10:14 PM > > To: Runyan, Arthur J > > Cc: Vivi, Rodrigo ; Ville Syrjälä > > ; intel-gfx > > Subject: Re: [Regression] "drm/i915: Implement display w/a #1143" breaks > > HDMI on ASUS GL552VW > > > > Hi, > > > >> On Aug 14, 2020, at 01:56, Runyan, Arthur J > >> wrote: > >> > >> The workaround is freeing up stuck vswing values to let new vswing > >> programming kick in. Maybe the new vswing values are wrong. > >> Try checking the vswing that driver programs against what BIOS/GOP > >> programs. > > > > Do you mean to print out value of I915_READ()? > > val = I915_READ(CHICKEN_TRANS(transcoder)); > > > > Kai-Heng > > > >> > >> -Original Message- > >> From: Vivi, Rodrigo > >> Sent: Thursday, August 13, 2020 9:50 AM > >> To: Kai-Heng Feng ; Runyan, Arthur J > >> > >> Cc: Ville Syrjälä ; intel-gfx > >> > >> Subject: Re: [Regression] "drm/i915: Implement display w/a #1143" > >> breaks HDMI on ASUS GL552VW > >> > >> Art, any comment here? > >> > >> I just checked and the W/a 1143 is implemented as described, but it is > >> failing HDMI on this hybrid system. > >> > >>> On Aug 12, 2020, at 9:07 PM, Kai-Heng Feng > >>> wrote: > >>> > >>> Hi, > >>> > >>> There's a regression reported that HDMI output stops working after os > >>> upgrade: > >>> https://bugs.launchpad.net/bugs/1871721 > >>> > >>> Here's the bisect result: > >>> 0519c102f5285476d7868a387bdb6c58385e4074 is the first bad commit > >>> commit 0519c102f5285476d7868a387bdb6c58385e4074 > >>> Author: Ville Syrjälä > >>> Date: Mon Jan 22 19:41:31 2018 +0200 > >>> > >>> drm/i915: Implement display w/a #1143 > >>> > >>> Apparently SKL/KBL/CFL need some manual help to get the > >>> programmed HDMI vswing to stick. Implement the relevant > >>> workaround (display w/a #1143). > >>> > >>> Note that the relevant chicken bits live in a transcoder register > >>> even though the bits affect a specific DDI port rather than a > >>> specific transcoder. Hence we must pick the correct transcoder > >>> register instance based on the port rather than based on the > >>> cpu_transcoder. > >>> > >>> Also note that for completeness I included support for DDI A/E > >>> in the code even though we never have HDMI on those ports. > >>> > >>> v2: CFL needs the w/a as well (Rodrigo and Art) > >>> > >>> Cc: Rodrigo Vivi > >>> Cc: Art Runyan > >>> Signed-off-by: Ville Syrjälä > >>> Link: > >>> https://patchwork.freedesktop.org/patch/msgid/20180122174131.28046-1-ville.syrj...@linux.intel.com > >>> Reviewed-by: Rodrigo Vivi > >>> > >>> > >>> dmesg from drm-tip with drm.debug=0xe can be found here: > >>> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1871721/comments > >>> / > >>> 64 > >>> > >>> Kai-Heng > >> > >> > > -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/gt: Show engine properties in the pretty printer
== Series Details == Series: drm/i915/gt: Show engine properties in the pretty printer URL : https://patchwork.freedesktop.org/series/80947/ State : failure == Summary == CI Bug Log - changes from CI_DRM_8918_full -> Patchwork_18394_full Summary --- **FAILURE** Serious unknown changes coming with Patchwork_18394_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_18394_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_18394_full: ### IGT changes ### Possible regressions * igt@gem_busy@close-race: - shard-hsw: [PASS][1] -> [INCOMPLETE][2] +1 similar issue [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-hsw8/igt@gem_b...@close-race.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18394/shard-hsw7/igt@gem_b...@close-race.html Known issues Here are the changes found in Patchwork_18394_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_exec_whisper@basic-queues-forked: - shard-glk: [PASS][3] -> [DMESG-WARN][4] ([i915#118] / [i915#95]) +1 similar issue [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-glk7/igt@gem_exec_whis...@basic-queues-forked.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18394/shard-glk3/igt@gem_exec_whis...@basic-queues-forked.html * igt@i915_selftest@live@execlists: - shard-apl: [PASS][5] -> [INCOMPLETE][6] ([i915#1635]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-apl2/igt@i915_selftest@l...@execlists.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18394/shard-apl6/igt@i915_selftest@l...@execlists.html * igt@kms_big_fb@linear-64bpp-rotate-0: - shard-glk: [PASS][7] -> [DMESG-FAIL][8] ([i915#118] / [i915#95]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-glk1/igt@kms_big...@linear-64bpp-rotate-0.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18394/shard-glk8/igt@kms_big...@linear-64bpp-rotate-0.html * igt@kms_big_fb@x-tiled-8bpp-rotate-0: - shard-apl: [PASS][9] -> [DMESG-WARN][10] ([i915#1635] / [i915#1982]) +1 similar issue [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-apl8/igt@kms_big...@x-tiled-8bpp-rotate-0.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18394/shard-apl1/igt@kms_big...@x-tiled-8bpp-rotate-0.html * igt@kms_draw_crc@draw-method-xrgb2101010-blt-xtiled: - shard-kbl: [PASS][11] -> [DMESG-WARN][12] ([i915#1982]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-kbl1/igt@kms_draw_...@draw-method-xrgb2101010-blt-xtiled.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18394/shard-kbl7/igt@kms_draw_...@draw-method-xrgb2101010-blt-xtiled.html * igt@kms_frontbuffer_tracking@fbc-suspend: - shard-kbl: [PASS][13] -> [DMESG-WARN][14] ([i915#180]) +9 similar issues [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-kbl7/igt@kms_frontbuffer_track...@fbc-suspend.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18394/shard-kbl4/igt@kms_frontbuffer_track...@fbc-suspend.html * igt@kms_frontbuffer_tracking@psr-1p-primscrn-indfb-plflip-blt: - shard-tglb: [PASS][15] -> [DMESG-WARN][16] ([i915#1982]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-tglb3/igt@kms_frontbuffer_track...@psr-1p-primscrn-indfb-plflip-blt.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18394/shard-tglb2/igt@kms_frontbuffer_track...@psr-1p-primscrn-indfb-plflip-blt.html * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc: - shard-skl: [PASS][17] -> [FAIL][18] ([fdo#108145] / [i915#265]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-skl6/igt@kms_plane_alpha_bl...@pipe-b-coverage-7efc.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18394/shard-skl3/igt@kms_plane_alpha_bl...@pipe-b-coverage-7efc.html * igt@kms_plane_scaling@pipe-a-scaler-with-clipping-clamping: - shard-iclb: [PASS][19] -> [DMESG-WARN][20] ([i915#1982]) +1 similar issue [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8918/shard-iclb2/igt@kms_plane_scal...@pipe-a-scaler-with-clipping-clamping.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18394/shard-iclb3/igt@kms_plane_scal...@pipe-a-scaler-with-clipping-clamping.html * igt@kms_plane_scaling@pipe-c-plane-scaling: - shard-skl: [PASS][21] -> [DMESG-WARN][22] ([i915#1982]) +11 similar issues [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_
Re: [Intel-gfx] [PATCH] drm/i915/display: Add an extra vblank wait before fbc activation
On Mon, Aug 17, 2020 at 01:14:18PM +0530, Uma Shankar wrote:
> Add an extra vblank before fbc is activated.
> WA: 1409689360
> Corruption with FBC around plane 1A enabling. In the Frame Buffer
> Compression programming sequence "Display Plane Enabling with FBC"
> add a wait for vblank between plane enabling step 1 and FBC enabling
> step 2.
Already there due to drm_atomic_helper_wait_for_flip_done().
>
> Signed-off-by: Uma Shankar
> Signed-off-by: Stanislav Lisovskiy
> ---
> drivers/gpu/drm/i915/display/intel_fbc.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c
> b/drivers/gpu/drm/i915/display/intel_fbc.c
> index 2ab32e6532ff..0ed252ff2c53 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -1085,10 +1085,12 @@ static void __intel_fbc_post_update(struct intel_crtc
> *crtc)
> if (!intel_fbc_can_activate(crtc))
> return;
>
> - if (!fbc->busy_bits)
> + if (!fbc->busy_bits) {
> + intel_wait_for_vblank(dev_priv, crtc->pipe);
> intel_fbc_hw_activate(dev_priv);
> - else
> + } else {
> intel_fbc_deactivate(dev_priv, "frontbuffer write");
> + }
> }
>
> void intel_fbc_post_update(struct intel_atomic_state *state,
> --
> 2.22.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Regression] "drm/i915: Implement display w/a #1143" breaks HDMI on ASUS GL552VW
I remember some strangeness about the blnclegdisbl. I'll see if I can dig up some more. -Original Message- From: Ville Syrjälä Sent: Monday, August 24, 2020 11:05 AM To: Kai-Heng Feng Cc: Runyan, Arthur J ; Vivi, Rodrigo ; intel-gfx Subject: Re: [Regression] "drm/i915: Implement display w/a #1143" breaks HDMI on ASUS GL552VW On Mon, Aug 17, 2020 at 02:17:49PM +0800, Kai-Heng Feng wrote: > > > > On Aug 17, 2020, at 00:22, Runyan, Arthur J > > wrote: > > > > You'll need to read out the DDI_BUF_TRANS_* and DISPIO_CR_TX_BMU_CR0 > > registers at boot before i915 programs them and compare with what driver > > programs. > > Rodrigo can probably show you how. > > Right, I'll wait for a patch then :) To grab the BIOS reg values we just have to make sure the driver doesn't load. Eg. pass something like "modprobe.blacklist=i915,snd_hda_intel 3" to the kernel cmdline (+ whatever other magic ubuntu might require). Confirm with something like "lsmod | grep i915" to make sure the driver didn't sneak in despite our best efforts. Then we can dump the registers with intel_reg from igt-gpu-tools: intel_reg read --count 20 0x64E00 0x64E60 0x64EC0 0x64F20 0x64F80 intel_reg read 0x64000 0x64100 0x64200 0x64300 0x64400 0x6C00C The only somewhat suspicious thing I noticed is that we treat DISPIO_CR_TX_BMU_CR0:tx_blnclegdisbl as a bitmask (bit 23 -> DDI A, bit 24 -> DDI B, etc.) whereas the spec seems to be saying that we should just zero out all the bits of tx_blnclegdisbl when any DDI needs iboost. Art, is our interpretation of the bits correct or just a fairy tale? > > Kai-Heng > > > > > -Original Message- > > From: Kai-Heng Feng > > Sent: Thursday, August 13, 2020 10:14 PM > > To: Runyan, Arthur J > > Cc: Vivi, Rodrigo ; Ville Syrjälä > > ; intel-gfx > > > > Subject: Re: [Regression] "drm/i915: Implement display w/a #1143" > > breaks HDMI on ASUS GL552VW > > > > Hi, > > > >> On Aug 14, 2020, at 01:56, Runyan, Arthur J > >> wrote: > >> > >> The workaround is freeing up stuck vswing values to let new vswing > >> programming kick in. Maybe the new vswing values are wrong. > >> Try checking the vswing that driver programs against what BIOS/GOP > >> programs. > > > > Do you mean to print out value of I915_READ()? > > val = I915_READ(CHICKEN_TRANS(transcoder)); > > > > Kai-Heng > > > >> > >> -Original Message- > >> From: Vivi, Rodrigo > >> Sent: Thursday, August 13, 2020 9:50 AM > >> To: Kai-Heng Feng ; Runyan, Arthur J > >> > >> Cc: Ville Syrjälä ; intel-gfx > >> > >> Subject: Re: [Regression] "drm/i915: Implement display w/a #1143" > >> breaks HDMI on ASUS GL552VW > >> > >> Art, any comment here? > >> > >> I just checked and the W/a 1143 is implemented as described, but it is > >> failing HDMI on this hybrid system. > >> > >>> On Aug 12, 2020, at 9:07 PM, Kai-Heng Feng > >>> wrote: > >>> > >>> Hi, > >>> > >>> There's a regression reported that HDMI output stops working after os > >>> upgrade: > >>> https://bugs.launchpad.net/bugs/1871721 > >>> > >>> Here's the bisect result: > >>> 0519c102f5285476d7868a387bdb6c58385e4074 is the first bad commit > >>> commit 0519c102f5285476d7868a387bdb6c58385e4074 > >>> Author: Ville Syrjälä > >>> Date: Mon Jan 22 19:41:31 2018 +0200 > >>> > >>> drm/i915: Implement display w/a #1143 > >>> > >>> Apparently SKL/KBL/CFL need some manual help to get the > >>> programmed HDMI vswing to stick. Implement the relevant > >>> workaround (display w/a #1143). > >>> > >>> Note that the relevant chicken bits live in a transcoder register > >>> even though the bits affect a specific DDI port rather than a > >>> specific transcoder. Hence we must pick the correct transcoder > >>> register instance based on the port rather than based on the > >>> cpu_transcoder. > >>> > >>> Also note that for completeness I included support for DDI A/E > >>> in the code even though we never have HDMI on those ports. > >>> > >>> v2: CFL needs the w/a as well (Rodrigo and Art) > >>> > >>> Cc: Rodrigo Vivi > >>> Cc: Art Runyan > >>> Signed-off-by: Ville Syrjälä > >>> Link: > >>> https://patchwork.freedesktop.org/patch/msgid/20180122174131.28046 > >>> -1-ville.syrj...@linux.intel.com > >>> Reviewed-by: Rodrigo Vivi > >>> > >>> > >>> dmesg from drm-tip with drm.debug=0xe can be found here: > >>> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1871721/comme > >>> nts > >>> / > >>> 64 > >>> > >>> Kai-Heng > >> > >> > > -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 05/22] drm/i915/dg1: Wait for pcode/uncore handshake at startup
On Mon, Aug 03, 2020 at 04:24:17PM -0700, Jose Souza wrote:
On Fri, 2020-07-24 at 14:39 -0700, Lucas De Marchi wrote:
From: Matt Roper <
matthew.d.ro...@intel.com
>
DG1 does some additional pcode/uncore handshaking at
boot time; this handshaking must complete before various other pcode
commands are effective and before general work is submitted to the GPU.
We need to poll a new pcode mailbox during startup until it reports that
this handshaking is complete.
The bspec doesn't give guidance on how long we may need to wait for this
handshaking to complete. For now, let's just set a really long timeout;
if we still don't get a completion status by the end of that timeout,
we'll just continue on and hope for the best.
Bspec: 52065
Cc: Clinton Taylor <
clinton.a.tay...@intel.com
>
Cc: Ville Syrjälä <
ville.syrj...@linux.intel.com
>
Cc: Radhakrishna Sripada <
radhakrishna.srip...@intel.com
>
Signed-off-by: Matt Roper <
matthew.d.ro...@intel.com
>
Signed-off-by: Lucas De Marchi <
lucas.demar...@intel.com
>
---
drivers/gpu/drm/i915/i915_drv.c | 3 +++
drivers/gpu/drm/i915/i915_reg.h | 3 +++
drivers/gpu/drm/i915/intel_sideband.c | 15 +++
drivers/gpu/drm/i915/intel_sideband.h | 2 ++
4 files changed, 23 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5fd5af4bc855..5473bfe9126c 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -85,6 +85,7 @@
#include "intel_gvt.h"
#include "intel_memory_region.h"
#include "intel_pm.h"
+#include "intel_sideband.h"
#include "vlv_suspend.h"
static struct drm_driver driver;
@@ -737,6 +738,8 @@ static int i915_driver_hw_probe(struct drm_i915_private
*dev_priv)
*/
intel_dram_detect(dev_priv);
+ intel_pcode_init(dev_priv);
+
intel_bw_init_hw(dev_priv);
return 0;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a0d31f3bf634..3767b32127da 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -9245,6 +9245,9 @@ enum {
#define GEN9_SAGV_DISABLE 0x0
#define GEN9_SAGV_IS_DISABLED 0x1
#define GEN9_SAGV_ENABLE 0x3
+#define DG1_PCODE_STATUS 0x7E
+#define DG1_CHECK_UNCORE_INIT_STATUS 0x0
+#define DG1_UNCORE_INIT_COMPLETE 0x1
With s/DG1_CHECK_UNCORE_INIT_STATUS/DG1_CHECK_UNCORE_INIT_STATUS_COMPLETE or
something similar that makes easy to understand that 0x1 is the response
of the DG1_CHECK_UNCORE_INIT_STATUS sub-command.
checking all the other users of skl_pcode_request() I don't see a
pattern there. Examples:
ret = skl_pcode_request(dev_priv, SKL_PCODE_CDCLK_CONTROL,
SKL_CDCLK_PREPARE_FOR_CHANGE,
SKL_CDCLK_READY_FOR_CHANGE,
SKL_CDCLK_READY_FOR_CHANGE, 3);
ret = skl_pcode_request(dev_priv, GEN9_PCODE_SAGV_CONTROL,
GEN9_SAGV_DISABLE,
GEN9_SAGV_IS_DISABLED, GEN9_SAGV_IS_DISABLED,
1);
Giveng the current uses, I'd rather rename like:
+#define DG1_PCODE_STATUS 0x7E
+#define DG1_UNCORE_GET_INIT_STATUS 0x0
+#define DG1_UNCORE_INIT_STATUS_COMPLETE0x1
Reviewed-by: José Roberto de Souza
does that still stands with the rename above?
thanks
Lucas De Marchi
#define GEN12_PCODE_READ_SAGV_BLOCK_TIME_US0x23
#define GEN6_PCODE_DATA_MMIO(0x138128)
#define GEN6_PCODE_FREQ_IA_RATIO_SHIFT 8
diff --git a/drivers/gpu/drm/i915/intel_sideband.c
b/drivers/gpu/drm/i915/intel_sideband.c
index 916ccd1c0e96..8b093525240d 100644
--- a/drivers/gpu/drm/i915/intel_sideband.c
+++ b/drivers/gpu/drm/i915/intel_sideband.c
@@ -543,3 +543,18 @@ int skl_pcode_request(struct drm_i915_private *i915, u32
mbox, u32 request,
return ret ? ret : status;
#undef COND
}
+
+void intel_pcode_init(struct drm_i915_private *i915)
+{
+ int ret;
+
+ if (!IS_DGFX(i915))
+ return;
+
+ ret = skl_pcode_request(i915, DG1_PCODE_STATUS,
+ DG1_CHECK_UNCORE_INIT_STATUS,
+ DG1_UNCORE_INIT_COMPLETE,
+ DG1_UNCORE_INIT_COMPLETE, 50);
+ if (ret)
+ drm_err(&i915->drm, "Pcode did not report uncore initialization
completion!\n");
+}
diff --git a/drivers/gpu/drm/i915/intel_sideband.h
b/drivers/gpu/drm/i915/intel_sideband.h
index 7fb95745a444..094c7b19c5d4 100644
--- a/drivers/gpu/drm/i915/intel_sideband.h
+++ b/drivers/gpu/drm/i915/intel_sideband.h
@@ -138,4 +138,6 @@ int sandybridge_pcode_write_timeout(struct drm_i915_private
*i915, u32 mbox,
int skl_pcode_request(struct drm_i915_private *i915, u32 mbox, u32 request,
Re: [Intel-gfx] [PATCH v5 05/22] drm/i915/dg1: Wait for pcode/uncore handshake at startup
On Mon, 2020-08-24 at 12:24 -0700, Lucas De Marchi wrote:
> On Mon, Aug 03, 2020 at 04:24:17PM -0700, Jose Souza wrote:
> > On Fri, 2020-07-24 at 14:39 -0700, Lucas De Marchi wrote:
> > > From: Matt Roper <
> > > matthew.d.ro...@intel.com
> > >
> > >
> > > DG1 does some additional pcode/uncore handshaking at
> > > boot time; this handshaking must complete before various other pcode
> > > commands are effective and before general work is submitted to the GPU.
> > > We need to poll a new pcode mailbox during startup until it reports that
> > > this handshaking is complete.
> > >
> > > The bspec doesn't give guidance on how long we may need to wait for this
> > > handshaking to complete. For now, let's just set a really long timeout;
> > > if we still don't get a completion status by the end of that timeout,
> > > we'll just continue on and hope for the best.
> > >
> > > Bspec: 52065
> > > Cc: Clinton Taylor <
> > > clinton.a.tay...@intel.com
> > >
> > >
> > > Cc: Ville Syrjälä <
> > > ville.syrj...@linux.intel.com
> > >
> > >
> > > Cc: Radhakrishna Sripada <
> > > radhakrishna.srip...@intel.com
> > >
> > >
> > > Signed-off-by: Matt Roper <
> > > matthew.d.ro...@intel.com
> > >
> > >
> > > Signed-off-by: Lucas De Marchi <
> > > lucas.demar...@intel.com
> > >
> > >
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.c | 3 +++
> > > drivers/gpu/drm/i915/i915_reg.h | 3 +++
> > > drivers/gpu/drm/i915/intel_sideband.c | 15 +++
> > > drivers/gpu/drm/i915/intel_sideband.h | 2 ++
> > > 4 files changed, 23 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > > b/drivers/gpu/drm/i915/i915_drv.c
> > > index 5fd5af4bc855..5473bfe9126c 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -85,6 +85,7 @@
> > > #include "intel_gvt.h"
> > > #include "intel_memory_region.h"
> > > #include "intel_pm.h"
> > > +#include "intel_sideband.h"
> > > #include "vlv_suspend.h"
> > >
> > > static struct drm_driver driver;
> > > @@ -737,6 +738,8 @@ static int i915_driver_hw_probe(struct
> > > drm_i915_private *dev_priv)
> > >*/
> > > intel_dram_detect(dev_priv);
> > >
> > > + intel_pcode_init(dev_priv);
> > > +
> > > intel_bw_init_hw(dev_priv);
> > >
> > > return 0;
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index a0d31f3bf634..3767b32127da 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -9245,6 +9245,9 @@ enum {
> > > #define GEN9_SAGV_DISABLE0x0
> > > #define GEN9_SAGV_IS_DISABLED0x1
> > > #define GEN9_SAGV_ENABLE 0x3
> > > +#define DG1_PCODE_STATUS 0x7E
> > > +#define DG1_CHECK_UNCORE_INIT_STATUS 0x0
> > > +#define DG1_UNCORE_INIT_COMPLETE 0x1
> >
> > With s/DG1_CHECK_UNCORE_INIT_STATUS/DG1_CHECK_UNCORE_INIT_STATUS_COMPLETE
> > or something similar that makes easy to understand that 0x1 is the response
> > of the DG1_CHECK_UNCORE_INIT_STATUS sub-command.
>
> checking all the other users of skl_pcode_request() I don't see a
> pattern there. Examples:
>
> ret = skl_pcode_request(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> SKL_CDCLK_PREPARE_FOR_CHANGE,
> SKL_CDCLK_READY_FOR_CHANGE,
> SKL_CDCLK_READY_FOR_CHANGE, 3);
>
> ret = skl_pcode_request(dev_priv, GEN9_PCODE_SAGV_CONTROL,
> GEN9_SAGV_DISABLE,
> GEN9_SAGV_IS_DISABLED, GEN9_SAGV_IS_DISABLED,
> 1);
>
> Giveng the current uses, I'd rather rename like:
>
> +#define DG1_PCODE_STATUS 0x7E
> +#define DG1_UNCORE_GET_INIT_STATUS 0x0
> +#define DG1_UNCORE_INIT_STATUS_COMPLETE 0x1
>
>
> > Reviewed-by: José Roberto de Souza <
> > jose.so...@intel.com
> > >
>
> does that still stands with the rename above?
LGTM, keep it please.
>
> thanks
> Lucas De Marchi
>
> >
> > > #define GEN12_PCODE_READ_SAGV_BLOCK_TIME_US 0x23
> > > #define GEN6_PCODE_DATA _MMIO(0x138128)
> > > #define GEN6_PCODE_FREQ_IA_RATIO_SHIFT 8
> > > diff --git a/drivers/gpu/drm/i915/intel_sideband.c
> > > b/drivers/gpu/drm/i915/intel_sideband.c
> > > index 916ccd1c0e96..8b093525240d 100644
> > > --- a/drivers/gpu/drm/i915/intel_sideband.c
> > > +++ b/drivers/gpu/drm/i915/intel_sideband.c
> > > @@ -543,3 +543,18 @@ int skl_pcode_request(struct drm_i915_private *i915,
> > > u32 mbox, u32 request,
> > > return ret ? ret : status;
> > > #undef COND
> > > }
> > > +
> > > +void intel_pcode_init(struct drm_i915_private *i915)
> > > +{
> > > + int ret;
> > > +
> > > + if (!IS_DGFX(i915))
> > > + return;
> > > +
> > > + ret = skl_pcode_request(i91
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915/display: Compute has_drrs after compute has_psr
== Series Details == Series: series starting with [v2,1/3] drm/i915/display: Compute has_drrs after compute has_psr URL : https://patchwork.freedesktop.org/series/80953/ State : success == Summary == CI Bug Log - changes from CI_DRM_8921 -> Patchwork_18395 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18395/index.html Known issues Here are the changes found in Patchwork_18395 that come from known issues: ### IGT changes ### Issues hit * igt@i915_pm_rpm@module-reload: - fi-byt-j1900: [PASS][1] -> [DMESG-WARN][2] ([i915#1982]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8921/fi-byt-j1900/igt@i915_pm_...@module-reload.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18395/fi-byt-j1900/igt@i915_pm_...@module-reload.html * igt@kms_flip@basic-flip-vs-wf_vblank@c-hdmi-a2: - fi-skl-guc: [PASS][3] -> [DMESG-WARN][4] ([i915#2203]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8921/fi-skl-guc/igt@kms_flip@basic-flip-vs-wf_vbl...@c-hdmi-a2.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18395/fi-skl-guc/igt@kms_flip@basic-flip-vs-wf_vbl...@c-hdmi-a2.html Possible fixes * igt@gem_exec_suspend@basic-s3: - fi-tgl-u2: [FAIL][5] ([i915#1888]) -> [PASS][6] [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8921/fi-tgl-u2/igt@gem_exec_susp...@basic-s3.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18395/fi-tgl-u2/igt@gem_exec_susp...@basic-s3.html Warnings * igt@i915_pm_rpm@module-reload: - fi-kbl-x1275: [SKIP][7] ([fdo#109271]) -> [DMESG-FAIL][8] ([i915#62] / [i915#95]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8921/fi-kbl-x1275/igt@i915_pm_...@module-reload.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18395/fi-kbl-x1275/igt@i915_pm_...@module-reload.html * igt@kms_force_connector_basic@force-edid: - fi-kbl-x1275: [DMESG-WARN][9] ([i915#62] / [i915#92]) -> [DMESG-WARN][10] ([i915#62] / [i915#92] / [i915#95]) +2 similar issues [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8921/fi-kbl-x1275/igt@kms_force_connector_ba...@force-edid.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18395/fi-kbl-x1275/igt@kms_force_connector_ba...@force-edid.html * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a: - fi-kbl-x1275: [DMESG-WARN][11] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][12] ([i915#62] / [i915#92]) +7 similar issues [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8921/fi-kbl-x1275/igt@kms_pipe_crc_ba...@suspend-read-crc-pipe-a.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18395/fi-kbl-x1275/igt@kms_pipe_crc_ba...@suspend-read-crc-pipe-a.html [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271 [i915#1888]: https://gitlab.freedesktop.org/drm/intel/issues/1888 [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982 [i915#2203]: https://gitlab.freedesktop.org/drm/intel/issues/2203 [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62 [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92 [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95 Participating hosts (37 -> 34) -- Missing(3): fi-byt-clapper fi-byt-squawks fi-bsw-cyan Build changes - * Linux: CI_DRM_8921 -> Patchwork_18395 CI-20190529: 20190529 CI_DRM_8921: c47b8075ca959f650af549bf50c820c6b69d9af8 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_5770: f1d0c240ea2e631dfb9f493f37f8fb61cb2b1cf2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_18395: 1ebafa99bce6a46215a429e5eabb4409f8aea09f @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 1ebafa99bce6 drm/i915/display: Fix DRRS debugfs 6b7a2c9ad762 drm/i915/display: Disable DRRS when needed in fastsets 6c122a87d8a8 drm/i915/display: Compute has_drrs after compute has_psr == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18395/index.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/display: Add an extra vblank wait before fbc activation
> -Original Message-
> From: Ville Syrjälä
> Sent: Monday, August 24, 2020 11:46 PM
> To: Shankar, Uma
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Add an extra vblank wait
> before fbc activation
>
> On Mon, Aug 17, 2020 at 01:14:18PM +0530, Uma Shankar wrote:
> > Add an extra vblank before fbc is activated.
> > WA: 1409689360
> > Corruption with FBC around plane 1A enabling. In the Frame Buffer
> > Compression programming sequence "Display Plane Enabling with FBC"
> > add a wait for vblank between plane enabling step 1 and FBC enabling
> > step 2.
>
> Already there due to drm_atomic_helper_wait_for_flip_done().
Hi Ville,
__intel_fbc_post_update is also called through intel_fbc_flush. The extra wait
at that point seem
to be taking care of this case as well.
We can add it in vblank worker as suggested by Maarten or do you feel this
should be handled differently.
Regards,
Uma Shankar
> >
> > Signed-off-by: Uma Shankar
> > Signed-off-by: Stanislav Lisovskiy
> > ---
> > drivers/gpu/drm/i915/display/intel_fbc.c | 6 --
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c
> > b/drivers/gpu/drm/i915/display/intel_fbc.c
> > index 2ab32e6532ff..0ed252ff2c53 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> > @@ -1085,10 +1085,12 @@ static void __intel_fbc_post_update(struct
> intel_crtc *crtc)
> > if (!intel_fbc_can_activate(crtc))
> > return;
> >
> > - if (!fbc->busy_bits)
> > + if (!fbc->busy_bits) {
> > + intel_wait_for_vblank(dev_priv, crtc->pipe);
> > intel_fbc_hw_activate(dev_priv);
> > - else
> > + } else {
> > intel_fbc_deactivate(dev_priv, "frontbuffer write");
> > + }
> > }
> >
> > void intel_fbc_post_update(struct intel_atomic_state *state,
> > --
> > 2.22.0
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/5] Fixing Possible Null Pointer Dereference
On Mon, Aug 24, 2020 at 09:15:52AM +0530, Nischal Varide wrote: > There is a possble Null Pointer dereference in intel_atomic.c and this > patch fixes the same by introducting a check to old_state, new_state > old_conn_state and new_conn_state variables. Not possible. In fact none of the supposed null ptrs in the series seem possible to me. > > Signed-off-by: Nischal Varide > --- > drivers/gpu/drm/i915/display/intel_atomic.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c > b/drivers/gpu/drm/i915/display/intel_atomic.c > index 630f49b7aa01..ab58f061c8a7 100644 > --- a/drivers/gpu/drm/i915/display/intel_atomic.c > +++ b/drivers/gpu/drm/i915/display/intel_atomic.c > @@ -132,6 +132,9 @@ int intel_digital_connector_atomic_check(struct > drm_connector *conn, > to_intel_digital_connector_state(old_state); > struct drm_crtc_state *crtc_state; > > + if (!(new_state && new_conn_state && old_state && old_conn_state)) > + return 0; > + > intel_hdcp_atomic_check(conn, old_state, new_state); > intel_psr_atomic_check(conn, old_state, new_state); > > @@ -192,6 +195,8 @@ intel_connector_needs_modeset(struct intel_atomic_state > *state, > > old_conn_state = drm_atomic_get_old_connector_state(&state->base, > connector); > new_conn_state = drm_atomic_get_new_connector_state(&state->base, > connector); > + if (!(old_conn_state && new_conn_state)) > + return 0; > > return old_conn_state->crtc != new_conn_state->crtc || > (new_conn_state->crtc && > -- > 2.26.0 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/display: Add an extra vblank wait before fbc activation
On Mon, Aug 24, 2020 at 07:46:30PM +, Shankar, Uma wrote:
>
> > -Original Message-
> > From: Ville Syrjälä
> > Sent: Monday, August 24, 2020 11:46 PM
> > To: Shankar, Uma
> > Cc: intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Add an extra vblank wait
> > before fbc activation
> >
> > On Mon, Aug 17, 2020 at 01:14:18PM +0530, Uma Shankar wrote:
> > > Add an extra vblank before fbc is activated.
> > > WA: 1409689360
> > > Corruption with FBC around plane 1A enabling. In the Frame Buffer
> > > Compression programming sequence "Display Plane Enabling with FBC"
> > > add a wait for vblank between plane enabling step 1 and FBC enabling
> > > step 2.
> >
> > Already there due to drm_atomic_helper_wait_for_flip_done().
>
> Hi Ville,
> __intel_fbc_post_update is also called through intel_fbc_flush. The extra
> wait at that point seem
> to be taking care of this case as well.
>
> We can add it in vblank worker as suggested by Maarten or do you feel this
> should be handled differently.
There's already supposed to be something that prevents the frontbuffer
stuff from racing with plane updates.
>
> Regards,
> Uma Shankar
>
> > >
> > > Signed-off-by: Uma Shankar
> > > Signed-off-by: Stanislav Lisovskiy
> > > ---
> > > drivers/gpu/drm/i915/display/intel_fbc.c | 6 --
> > > 1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c
> > > b/drivers/gpu/drm/i915/display/intel_fbc.c
> > > index 2ab32e6532ff..0ed252ff2c53 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> > > @@ -1085,10 +1085,12 @@ static void __intel_fbc_post_update(struct
> > intel_crtc *crtc)
> > > if (!intel_fbc_can_activate(crtc))
> > > return;
> > >
> > > - if (!fbc->busy_bits)
> > > + if (!fbc->busy_bits) {
> > > + intel_wait_for_vblank(dev_priv, crtc->pipe);
> > > intel_fbc_hw_activate(dev_priv);
> > > - else
> > > + } else {
> > > intel_fbc_deactivate(dev_priv, "frontbuffer write");
> > > + }
> > > }
> > >
> > > void intel_fbc_post_update(struct intel_atomic_state *state,
> > > --
> > > 2.22.0
> > >
> > > ___
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Ville Syrjälä
> > Intel
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/dp: DPTX writes Swing/Pre-emphs(DPCD 0x103-0x106) requested during PHY Layer testing.
On Fri, Aug 21, 2020 at 11:48:37PM -0700, Khaled Almahallawy wrote: > Source needs to write DPCD 103-106 after receiving a PHY request to change > swing/pre-emphasis after reading DPCD 206-207. This is especially needed if > there is a retimer between source and sink and the retimer implements AUX_CH > interception scheme to manage DP PHY settings (e.g. adjusting > Swing/Pre-emphasis > equalization level) for DP output channel . If the source doesn't write to > DPCD 103-106, the retimer may not output the requested swing/pre-emphasis and > eventually we fail compliance. > > Signed-off-by: Khaled Almahallawy > --- > drivers/gpu/drm/i915/display/intel_dp.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > b/drivers/gpu/drm/i915/display/intel_dp.c > index 79c27f91f42c..5044201ca742 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -5503,6 +5503,9 @@ void intel_dp_process_phy_request(struct intel_dp > *intel_dp) > > intel_dp_autotest_phy_ddi_enable(intel_dp, data->num_lanes); > > + drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_LANE0_SET, > + intel_dp->train_set, intel_dp->lane_count); Does this also help with spitting out the correct voltage levels during the actual training Does this fix LT failure seen on Type C ports with retimers? Manasi > + > drm_dp_set_phy_test_pattern(&intel_dp->aux, data, > link_status[DP_DPCD_REV]); > } > -- > 2.17.1 > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 21/22] drm/i915/dg1: DG1 does not support DC6
On Mon, Aug 03, 2020 at 04:33:45PM -0700, Jose Souza wrote:
On Fri, 2020-07-24 at 14:39 -0700, Lucas De Marchi wrote:
From: Anshuman Gupta <
anshuman.gu...@intel.com
>
DC6 is not supported on DG1, so change the allowed DC mask for DG1.
Cc: Uma Shankar <
uma.shan...@intel.com
>
Signed-off-by: Anshuman Gupta <
anshuman.gu...@intel.com
>
---
drivers/gpu/drm/i915/display/intel_display_power.c | 5 -
1 file 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 21f39c94056e..389a0f2d3a14 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -4689,7 +4689,10 @@ static u32 get_allowed_dc_mask(const struct
drm_i915_private *dev_priv,
int max_dc;
if (INTEL_GEN(dev_priv) >= 12) {
- max_dc = 4;
+ if (IS_DG1(dev_priv))
Better change to IS_DGFX() as DC6 is a SOC power-saving state, no discrete card
will enter it.
With this change:
that doesn't seem true... it's more a dg1 thing than general dgfx
Lucas De Marchi
Reviewed-by: José Roberto de Souza
+ max_dc = 3;
+ else
+ max_dc = 4;
/*
* DC9 has a separate HW flow from the rest of the DC states,
* not depending on the DMC firmware. It's needed by system
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 10/11] drm/i915: Add intel_update_bigjoiner handling.
On Wed, Jul 15, 2020 at 03:42:21PM -0700, Manasi Navare wrote:
> From: Maarten Lankhorst
>
> Enabling is done in a special sequence and so should plane updates
> be. Ideally the end user never notices the second pipe is used,
> so use the vblank evasion to cover both pipes.
>
> This way ideally everything will be tear free, and updates are
> really atomic as userspace expects it.
>
> This needs to be checked if it still works since lot of refactoring
> in skl_commit_modeset_enables
>
> v2:
> * Manual Rebase (Manasi)
> * Refactoring on intel_update_crtc and enable_crtc and removing
> special trans_port_sync_update (Manasi)
>
> Signed-off-by: Maarten Lankhorst
> Signed-off-by: Manasi Navare
This looks good to me in terms of modeset enable sequence as per Bspec
but would be good to get an ack from Ville/Maarten just to double
check I didnt break anything while rebase
Reviewed-by: Manasi Navare
Manasi
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 120 +--
> drivers/gpu/drm/i915/display/intel_sprite.c | 25 +++-
> drivers/gpu/drm/i915/display/intel_sprite.h | 3 +-
> 3 files changed, 129 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index a1011414da6d..00b26863ffc6 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -15656,7 +15656,7 @@ static void intel_update_crtc(struct
> intel_atomic_state *state,
> else
> i9xx_update_planes_on_crtc(state, crtc);
>
> - intel_pipe_update_end(new_crtc_state);
> + intel_pipe_update_end(new_crtc_state, NULL);
>
> /*
>* We usually enable FIFO underrun interrupts as part of the
> @@ -15754,6 +15754,52 @@ static void intel_commit_modeset_disables(struct
> intel_atomic_state *state)
> }
> }
>
> +static void intel_update_bigjoiner(struct intel_crtc *crtc,
> +struct intel_atomic_state *state,
> +struct intel_crtc_state *old_crtc_state,
> +struct intel_crtc_state *new_crtc_state)
> +{
> + struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> + bool modeset = needs_modeset(new_crtc_state);
> + struct intel_crtc *slave = new_crtc_state->bigjoiner_linked_crtc;
> + struct intel_crtc_state *new_slave_crtc_state =
> + intel_atomic_get_new_crtc_state(state, slave);
> +
> + if (modeset) {
> + /* Enable slave first */
> + intel_crtc_update_active_timings(new_slave_crtc_state);
> + dev_priv->display.crtc_enable(state, slave);
> +
> + /* Then master */
> + intel_crtc_update_active_timings(new_crtc_state);
> + dev_priv->display.crtc_enable(state, crtc);
> +
> + /* vblanks work again, re-enable pipe CRC. */
> + intel_crtc_enable_pipe_crc(crtc);
> +
> + } else {
> + intel_pre_plane_update(state, crtc);
> + intel_pre_plane_update(state, slave);
> +
> + if (new_crtc_state->update_pipe)
> + intel_encoders_update_pipe(state, crtc);
> + }
> +
> + /*
> + * Perform vblank evasion around commit operation, and make sure to
> + * commit both planes simultaneously for best results.
> + */
> + intel_pipe_update_start(new_crtc_state);
> +
> + commit_pipe_config(state, crtc);
> + commit_pipe_config(state, slave);
> +
> + skl_update_planes_on_crtc(state, crtc);
> + skl_update_planes_on_crtc(state, slave);
> +
> + intel_pipe_update_end(new_crtc_state, new_slave_crtc_state);
> +}
> +
> static void intel_commit_modeset_enables(struct intel_atomic_state *state)
> {
> struct intel_crtc_state *new_crtc_state;
> @@ -15772,15 +15818,22 @@ static void intel_commit_modeset_enables(struct
> intel_atomic_state *state)
> static void skl_commit_modeset_enables(struct intel_atomic_state *state)
> {
> struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> - struct intel_crtc *crtc;
> + struct intel_crtc *crtc, *slave;
> struct intel_crtc_state *old_crtc_state, *new_crtc_state;
> struct skl_ddb_entry entries[I915_MAX_PIPES] = {};
> + struct skl_ddb_entry new_entries[I915_MAX_PIPES] = {};
> u8 update_pipes = 0, modeset_pipes = 0;
> + const struct intel_crtc_state *slave_crtc_state;
> int i;
>
> for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> new_crtc_state, i) {
> enum pipe pipe = crtc->pipe;
>
> + if (new_crtc_state->bigjoiner_slave) {
> + /* We're updated from master */
> + continue;
> + }
> +
> if (!new_crtc_state->hw.active)
> continue;
>
> @@ -15791,6 +15844,34 @@ static void skl_commit_modeset_enables(struct
> intel
Re: [Intel-gfx] [PATCH 1/3] drm/i915/display/tgl: Use TGL DP tables for eDP ports without low power support
On Wed, Aug 19, 2020 at 11:51:44AM -0700, José Roberto de Souza wrote:
> Reusing icl_get_combo_buf_trans() for eDP was causing the wrong table
> being used when the eDP port don't support low power voltage swing table.
>
> Cc: Lee Shawn C
> Cc: Khaled Almahallawy
> Signed-off-by: José Roberto de Souza
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 52 +++-
> 1 file changed, 33 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index de5b216561d8..9a035bb7bd06 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -1088,30 +1088,44 @@ tgl_get_combo_buf_trans(struct intel_encoder
> *encoder, int type, int rate,
> {
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>
> - if (type == INTEL_OUTPUT_EDP && dev_priv->vbt.edp.hobl) {
> - struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> -
> - if (!intel_dp->hobl_failed && rate <= 54) {
> - /* Same table applies to TGL, RKL and DG1 */
> - *n_entries =
> ARRAY_SIZE(tgl_combo_phy_ddi_translations_edp_hbr2_hobl);
> - return tgl_combo_phy_ddi_translations_edp_hbr2_hobl;
> + switch (type) {
> + case INTEL_OUTPUT_HDMI:
> + *n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_hdmi);
> + return icl_combo_phy_ddi_translations_hdmi;
> + case INTEL_OUTPUT_EDP:
> + if (dev_priv->vbt.edp.hobl) {
> + struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> +
> + if (!intel_dp->hobl_failed && rate <= 54) {
> + /* Same table applies to TGL, RKL and DG1 */
> + *n_entries =
> ARRAY_SIZE(tgl_combo_phy_ddi_translations_edp_hbr2_hobl);
> + return
> tgl_combo_phy_ddi_translations_edp_hbr2_hobl;
> + }
> }
> - }
>
> - if (type == INTEL_OUTPUT_HDMI || type == INTEL_OUTPUT_EDP) {
> - return icl_get_combo_buf_trans(encoder, type, rate, n_entries);
> - } else if (rate > 27) {
> - if (IS_TGL_U(dev_priv) || IS_TGL_Y(dev_priv)) {
> - *n_entries =
> ARRAY_SIZE(tgl_uy_combo_phy_ddi_translations_dp_hbr2);
> - return tgl_uy_combo_phy_ddi_translations_dp_hbr2;
> + if (rate > 54) {
> + *n_entries =
> ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_hbr3);
> + return icl_combo_phy_ddi_translations_edp_hbr3;
So if we have (HBR3 && !low_vswing) we still want to use the eDP table
values? How did you figure that out? The only relevant comment I see
in the bspec is
eDP panels may support lower power, low voltage, swing values
using the "eDP" protocol values from the table or higher power,
high voltage, swing values using the "DP" protocol values.
which doesn't make any specific mention of HBR3 being a special case.
Matt
> + } else if (dev_priv->vbt.edp.low_vswing) {
> + *n_entries =
> ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_hbr2);
> + return icl_combo_phy_ddi_translations_edp_hbr2;
> + }
> + /* fall through */
> + default:
> + /* All combo DP and eDP ports that do not support low_vswing */
> + if (rate > 27) {
> + if (IS_TGL_U(dev_priv) || IS_TGL_Y(dev_priv)) {
> + *n_entries =
> ARRAY_SIZE(tgl_uy_combo_phy_ddi_translations_dp_hbr2);
> + return
> tgl_uy_combo_phy_ddi_translations_dp_hbr2;
> + }
> +
> + *n_entries =
> ARRAY_SIZE(tgl_combo_phy_ddi_translations_dp_hbr2);
> + return tgl_combo_phy_ddi_translations_dp_hbr2;
> }
>
> - *n_entries = ARRAY_SIZE(tgl_combo_phy_ddi_translations_dp_hbr2);
> - return tgl_combo_phy_ddi_translations_dp_hbr2;
> + *n_entries = ARRAY_SIZE(tgl_combo_phy_ddi_translations_dp_hbr);
> + return tgl_combo_phy_ddi_translations_dp_hbr;
> }
> -
> - *n_entries = ARRAY_SIZE(tgl_combo_phy_ddi_translations_dp_hbr);
> - return tgl_combo_phy_ddi_translations_dp_hbr;
> }
>
> static const struct tgl_dkl_phy_ddi_buf_trans *
> --
> 2.28.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915/display/tgl: Use TGL DP tables for eDP ports without low power support
On Mon, 2020-08-24 at 16:22 -0700, Matt Roper wrote:
> On Wed, Aug 19, 2020 at 11:51:44AM -0700, José Roberto de Souza wrote:
> > Reusing icl_get_combo_buf_trans() for eDP was causing the wrong table
> > being used when the eDP port don't support low power voltage swing table.
> >
> > Cc: Lee Shawn C <
> > shawn.c@intel.com
> > >
> > Cc: Khaled Almahallawy <
> > khaled.almahall...@intel.com
> > >
> > Signed-off-by: José Roberto de Souza <
> > jose.so...@intel.com
> > >
> > ---
> > drivers/gpu/drm/i915/display/intel_ddi.c | 52 +++-
> > 1 file changed, 33 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index de5b216561d8..9a035bb7bd06 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -1088,30 +1088,44 @@ tgl_get_combo_buf_trans(struct intel_encoder
> > *encoder, int type, int rate,
> > {
> > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >
> > - if (type == INTEL_OUTPUT_EDP && dev_priv->vbt.edp.hobl) {
> > - struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > -
> > - if (!intel_dp->hobl_failed && rate <= 54) {
> > - /* Same table applies to TGL, RKL and DG1 */
> > - *n_entries =
> > ARRAY_SIZE(tgl_combo_phy_ddi_translations_edp_hbr2_hobl);
> > - return tgl_combo_phy_ddi_translations_edp_hbr2_hobl;
> > + switch (type) {
> > + case INTEL_OUTPUT_HDMI:
> > + *n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_hdmi);
> > + return icl_combo_phy_ddi_translations_hdmi;
> > + case INTEL_OUTPUT_EDP:
> > + if (dev_priv->vbt.edp.hobl) {
> > + struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > +
> > + if (!intel_dp->hobl_failed && rate <= 54) {
> > + /* Same table applies to TGL, RKL and DG1 */
> > + *n_entries =
> > ARRAY_SIZE(tgl_combo_phy_ddi_translations_edp_hbr2_hobl);
> > + return
> > tgl_combo_phy_ddi_translations_edp_hbr2_hobl;
> > + }
> > }
> > - }
> >
> > - if (type == INTEL_OUTPUT_HDMI || type == INTEL_OUTPUT_EDP) {
> > - return icl_get_combo_buf_trans(encoder, type, rate, n_entries);
> > - } else if (rate > 27) {
> > - if (IS_TGL_U(dev_priv) || IS_TGL_Y(dev_priv)) {
> > - *n_entries =
> > ARRAY_SIZE(tgl_uy_combo_phy_ddi_translations_dp_hbr2);
> > - return tgl_uy_combo_phy_ddi_translations_dp_hbr2;
> > + if (rate > 54) {
> > + *n_entries =
> > ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_hbr3);
> > + return icl_combo_phy_ddi_translations_edp_hbr3;
>
> So if we have (HBR3 && !low_vswing) we still want to use the eDP table
> values? How did you figure that out? The only relevant comment I see
> in the bspec is
>
> eDP panels may support lower power, low voltage, swing values
> using the "eDP" protocol values from the table or higher power,
> high voltage, swing values using the "DP" protocol values.
>
> which doesn't make any specific mention of HBR3 being a special case.
As combo ports in DP mode don't support HBR3 it is trying to use HBR3 table
without even check if supported, in this case if the link training
fails intel_dp_get_link_train_fallback_values() will reduce the rate and lanes
until link training succeed or completely fails.
But looks unlikely that a vendor will be ship a product with a HBR3 eDP panel
and not set low_vswing in VBT.
>
>
> Matt
>
> > + } else if (dev_priv->vbt.edp.low_vswing) {
> > + *n_entries =
> > ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_hbr2);
> > + return icl_combo_phy_ddi_translations_edp_hbr2;
> > + }
> > + /* fall through */
> > + default:
> > + /* All combo DP and eDP ports that do not support low_vswing */
> > + if (rate > 27) {
> > + if (IS_TGL_U(dev_priv) || IS_TGL_Y(dev_priv)) {
> > + *n_entries =
> > ARRAY_SIZE(tgl_uy_combo_phy_ddi_translations_dp_hbr2);
> > + return
> > tgl_uy_combo_phy_ddi_translations_dp_hbr2;
> > + }
> > +
> > + *n_entries =
> > ARRAY_SIZE(tgl_combo_phy_ddi_translations_dp_hbr2);
> > + return tgl_combo_phy_ddi_translations_dp_hbr2;
> > }
> >
> > - *n_entries = ARRAY_SIZE(tgl_combo_phy_ddi_translations_dp_hbr2);
> > - return tgl_combo_phy_ddi_translations_dp_hbr2;
> > + *n_entries = ARRAY_SIZE(tgl_combo_phy_ddi_translations_dp_hbr);
> > + return tgl_combo_phy_ddi_translations_dp_hbr;
> > }
> > -
> > - *n_entries = ARRAY_SIZ
Re: [Intel-gfx] [PATCH 3/3] drm/i915/ehl: Update voltage swing table
On Wed, Aug 19, 2020 at 11:51:46AM -0700, José Roberto de Souza wrote:
> Update with latest tunning in the table.
>
> BSpec: 21257
> Signed-off-by: José Roberto de Souza
It looks like we now have separate tables for EHL and JSL; we probably
need to handle them with separate tables now.
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 0fa4075036e6..6271034455a8 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -572,14 +572,14 @@ static const struct cnl_ddi_buf_trans
> ehl_combo_phy_ddi_translations_dp[] = {
> /* NT mV Trans mV db*/
> { 0xA, 0x33, 0x3F, 0x00, 0x00 },/* 350 350 0.0 */
> { 0xA, 0x47, 0x36, 0x00, 0x09 },/* 350 500 3.1 */
> - { 0xC, 0x64, 0x30, 0x00, 0x0F },/* 350 700 6.0 */
> - { 0x6, 0x7F, 0x2C, 0x00, 0x13 },/* 350 900 8.2 */
> + { 0xC, 0x64, 0x34, 0x00, 0x0B },/* 350 700 6.0 */
> + { 0x6, 0x7F, 0x30, 0x00, 0x0F },/* 350 900 8.2 */
> { 0xA, 0x46, 0x3F, 0x00, 0x00 },/* 500 500 0.0 */
> - { 0xC, 0x64, 0x36, 0x00, 0x09 },/* 500 700 2.9 */
> - { 0x6, 0x7F, 0x30, 0x00, 0x0F },/* 500 900 5.1 */
> + { 0xC, 0x64, 0x38, 0x00, 0x07 },/* 500 700 2.9 */
> + { 0x6, 0x7F, 0x32, 0x00, 0x0D },/* 500 900 5.1 */
> { 0xC, 0x61, 0x3F, 0x00, 0x00 },/* 650 700 0.6 */
> - { 0x6, 0x7F, 0x37, 0x00, 0x08 },/* 600 900 3.5 */
> - { 0x6, 0x7F, 0x3F, 0x00, 0x00 },/* 900 900 0.0 */
> + { 0x6, 0x7F, 0x37, 0x00, 0x07 },/* 600 900 3.5 */
> + { 0x6, 0x7F, 0x38, 0x00, 0x00 },/* 900 900 0.0 */
These last two entries might have already changed again in the bspec
since you wrote this. For EHL I see:
0x6, 0x7f, 0x38, 0x00, 0x07
0x6, 0x7f, 0x3f, 0x00, 0x00
(i.e., the third column is slightly different). And the JSL table has
other deltas as well since it just got added.
Matt
> };
>
> struct icl_mg_phy_ddi_buf_trans {
> --
> 2.28.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915/display/tgl: Use TGL DP tables for eDP ports without low power support
On Mon, Aug 24, 2020 at 04:45:31PM -0700, Souza, Jose wrote:
> On Mon, 2020-08-24 at 16:22 -0700, Matt Roper wrote:
> > On Wed, Aug 19, 2020 at 11:51:44AM -0700, José Roberto de Souza wrote:
> > > Reusing icl_get_combo_buf_trans() for eDP was causing the wrong table
> > > being used when the eDP port don't support low power voltage swing table.
> > >
> > > Cc: Lee Shawn C <
> > > shawn.c@intel.com
> > > >
> > > Cc: Khaled Almahallawy <
> > > khaled.almahall...@intel.com
> > > >
> > > Signed-off-by: José Roberto de Souza <
> > > jose.so...@intel.com
> > > >
> > > ---
> > > drivers/gpu/drm/i915/display/intel_ddi.c | 52 +++-
> > > 1 file changed, 33 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > index de5b216561d8..9a035bb7bd06 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > @@ -1088,30 +1088,44 @@ tgl_get_combo_buf_trans(struct intel_encoder
> > > *encoder, int type, int rate,
> > > {
> > > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > >
> > > - if (type == INTEL_OUTPUT_EDP && dev_priv->vbt.edp.hobl) {
> > > - struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > > -
> > > - if (!intel_dp->hobl_failed && rate <= 54) {
> > > - /* Same table applies to TGL, RKL and DG1 */
> > > - *n_entries =
> > > ARRAY_SIZE(tgl_combo_phy_ddi_translations_edp_hbr2_hobl);
> > > - return tgl_combo_phy_ddi_translations_edp_hbr2_hobl;
> > > + switch (type) {
> > > + case INTEL_OUTPUT_HDMI:
> > > + *n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_hdmi);
> > > + return icl_combo_phy_ddi_translations_hdmi;
> > > + case INTEL_OUTPUT_EDP:
> > > + if (dev_priv->vbt.edp.hobl) {
> > > + struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > > +
> > > + if (!intel_dp->hobl_failed && rate <= 54) {
> > > + /* Same table applies to TGL, RKL and DG1 */
> > > + *n_entries =
> > > ARRAY_SIZE(tgl_combo_phy_ddi_translations_edp_hbr2_hobl);
> > > + return
> > > tgl_combo_phy_ddi_translations_edp_hbr2_hobl;
> > > + }
> > > }
> > > - }
> > >
> > > - if (type == INTEL_OUTPUT_HDMI || type == INTEL_OUTPUT_EDP) {
> > > - return icl_get_combo_buf_trans(encoder, type, rate, n_entries);
> > > - } else if (rate > 27) {
> > > - if (IS_TGL_U(dev_priv) || IS_TGL_Y(dev_priv)) {
> > > - *n_entries =
> > > ARRAY_SIZE(tgl_uy_combo_phy_ddi_translations_dp_hbr2);
> > > - return tgl_uy_combo_phy_ddi_translations_dp_hbr2;
> > > + if (rate > 54) {
> > > + *n_entries =
> > > ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_hbr3);
> > > + return icl_combo_phy_ddi_translations_edp_hbr3;
> >
> > So if we have (HBR3 && !low_vswing) we still want to use the eDP table
> > values? How did you figure that out? The only relevant comment I see
> > in the bspec is
> >
> > eDP panels may support lower power, low voltage, swing values
> > using the "eDP" protocol values from the table or higher power,
> > high voltage, swing values using the "DP" protocol values.
> >
> > which doesn't make any specific mention of HBR3 being a special case.
>
> As combo ports in DP mode don't support HBR3 it is trying to use HBR3
In that case should we drop the HBR3 check from the EHL patch? Because
on EHL the DP combo PHYs actually can support HBR3.
Matt
> table without even check if supported, in this case if the link
> training fails intel_dp_get_link_train_fallback_values() will reduce
> the rate and lanes until link training succeed or completely fails.
>
> But looks unlikely that a vendor will be ship a product with a HBR3
> eDP panel and not set low_vswing in VBT.
>
> >
> >
> > Matt
> >
> > > + } else if (dev_priv->vbt.edp.low_vswing) {
> > > + *n_entries =
> > > ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_hbr2);
> > > + return icl_combo_phy_ddi_translations_edp_hbr2;
> > > + }
> > > + /* fall through */
> > > + default:
> > > + /* All combo DP and eDP ports that do not support low_vswing */
> > > + if (rate > 27) {
> > > + if (IS_TGL_U(dev_priv) || IS_TGL_Y(dev_priv)) {
> > > + *n_entries =
> > > ARRAY_SIZE(tgl_uy_combo_phy_ddi_translations_dp_hbr2);
> > > + return
> > > tgl_uy_combo_phy_ddi_translations_dp_hbr2;
> > > + }
> > > +
> > > + *n_entries =
> > > ARRAY_SIZE(tgl_combo_phy_ddi_translations_dp_hbr2);
> > > + return tgl_combo_phy_ddi_translations_dp_hbr2;
> > > }
> > >
> > > -
Re: [Intel-gfx] [PATCH 1/3] drm/i915/display/tgl: Use TGL DP tables for eDP ports without low power support
On Mon, 2020-08-24 at 16:57 -0700, Matt Roper wrote:
> On Mon, Aug 24, 2020 at 04:45:31PM -0700, Souza, Jose wrote:
> > On Mon, 2020-08-24 at 16:22 -0700, Matt Roper wrote:
> > > On Wed, Aug 19, 2020 at 11:51:44AM -0700, José Roberto de Souza wrote:
> > > > Reusing icl_get_combo_buf_trans() for eDP was causing the wrong table
> > > > being used when the eDP port don't support low power voltage swing
> > > > table.
> > > >
> > > > Cc: Lee Shawn C <
> > > > shawn.c@intel.com
> > > >
> > > >
> > > > Cc: Khaled Almahallawy <
> > > > khaled.almahall...@intel.com
> > > >
> > > >
> > > > Signed-off-by: José Roberto de Souza <
> > > > jose.so...@intel.com
> > > >
> > > >
> > > > ---
> > > > drivers/gpu/drm/i915/display/intel_ddi.c | 52 +++-
> > > > 1 file changed, 33 insertions(+), 19 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > index de5b216561d8..9a035bb7bd06 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > @@ -1088,30 +1088,44 @@ tgl_get_combo_buf_trans(struct intel_encoder
> > > > *encoder, int type, int rate,
> > > > {
> > > > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > > >
> > > > - if (type == INTEL_OUTPUT_EDP && dev_priv->vbt.edp.hobl) {
> > > > - struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > > > -
> > > > - if (!intel_dp->hobl_failed && rate <= 54) {
> > > > - /* Same table applies to TGL, RKL and DG1 */
> > > > - *n_entries =
> > > > ARRAY_SIZE(tgl_combo_phy_ddi_translations_edp_hbr2_hobl);
> > > > - return
> > > > tgl_combo_phy_ddi_translations_edp_hbr2_hobl;
> > > > + switch (type) {
> > > > + case INTEL_OUTPUT_HDMI:
> > > > + *n_entries =
> > > > ARRAY_SIZE(icl_combo_phy_ddi_translations_hdmi);
> > > > + return icl_combo_phy_ddi_translations_hdmi;
> > > > + case INTEL_OUTPUT_EDP:
> > > > + if (dev_priv->vbt.edp.hobl) {
> > > > + struct intel_dp *intel_dp =
> > > > enc_to_intel_dp(encoder);
> > > > +
> > > > + if (!intel_dp->hobl_failed && rate <= 54) {
> > > > + /* Same table applies to TGL, RKL and
> > > > DG1 */
> > > > + *n_entries =
> > > > ARRAY_SIZE(tgl_combo_phy_ddi_translations_edp_hbr2_hobl);
> > > > + return
> > > > tgl_combo_phy_ddi_translations_edp_hbr2_hobl;
> > > > + }
> > > > }
> > > > - }
> > > >
> > > > - if (type == INTEL_OUTPUT_HDMI || type == INTEL_OUTPUT_EDP) {
> > > > - return icl_get_combo_buf_trans(encoder, type, rate,
> > > > n_entries);
> > > > - } else if (rate > 27) {
> > > > - if (IS_TGL_U(dev_priv) || IS_TGL_Y(dev_priv)) {
> > > > - *n_entries =
> > > > ARRAY_SIZE(tgl_uy_combo_phy_ddi_translations_dp_hbr2);
> > > > - return
> > > > tgl_uy_combo_phy_ddi_translations_dp_hbr2;
> > > > + if (rate > 54) {
> > > > + *n_entries =
> > > > ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_hbr3);
> > > > + return icl_combo_phy_ddi_translations_edp_hbr3;
> > >
> > > So if we have (HBR3 && !low_vswing) we still want to use the eDP table
> > > values? How did you figure that out? The only relevant comment I see
> > > in the bspec is
> > >
> > > eDP panels may support lower power, low voltage, swing values
> > > using the "eDP" protocol values from the table or higher power,
> > > high voltage, swing values using the "DP" protocol values.
> > >
> > > which doesn't make any specific mention of HBR3 being a special case.
> >
> > As combo ports in DP mode don't support HBR3 it is trying to use HBR3
>
> In that case should we drop the HBR3 check from the EHL patch? Because
> on EHL the DP combo PHYs actually can support HBR3.
Oh good catch will fix patch 3.
>
>
> Matt
>
> > table without even check if supported, in this case if the link
> > training fails intel_dp_get_link_train_fallback_values() will reduce
> > the rate and lanes until link training succeed or completely fails.
> >
> > But looks unlikely that a vendor will be ship a product with a HBR3
> > eDP panel and not set low_vswing in VBT.
> >
> > >
> > > Matt
> > >
> > > > + } else if (dev_priv->vbt.edp.low_vswing) {
> > > > + *n_entries =
> > > > ARRAY_SIZE(icl_combo_phy_ddi_translations_edp_hbr2);
> > > > + return icl_combo_phy_ddi_translations_edp_hbr2;
> > > > + }
> > > > + /* fall through */
> > > > + default:
> > > > + /* All combo
[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [v2,1/3] drm/i915/display: Compute has_drrs after compute has_psr
== Series Details == Series: series starting with [v2,1/3] drm/i915/display: Compute has_drrs after compute has_psr URL : https://patchwork.freedesktop.org/series/80953/ State : success == Summary == CI Bug Log - changes from CI_DRM_8921_full -> Patchwork_18395_full Summary --- **SUCCESS** No regressions found. Known issues Here are the changes found in Patchwork_18395_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_ctx_isolation@preservation-s3@vcs0: - shard-kbl: [PASS][1] -> [DMESG-WARN][2] ([i915#180]) +4 similar issues [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8921/shard-kbl4/igt@gem_ctx_isolation@preservation...@vcs0.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18395/shard-kbl2/igt@gem_ctx_isolation@preservation...@vcs0.html * igt@kms_big_fb@linear-64bpp-rotate-180: - shard-glk: [PASS][3] -> [DMESG-FAIL][4] ([i915#118] / [i915#95]) +1 similar issue [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8921/shard-glk9/igt@kms_big...@linear-64bpp-rotate-180.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18395/shard-glk8/igt@kms_big...@linear-64bpp-rotate-180.html * igt@kms_color@pipe-a-ctm-0-5: - shard-skl: [PASS][5] -> [DMESG-WARN][6] ([i915#1982]) +5 similar issues [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8921/shard-skl5/igt@kms_co...@pipe-a-ctm-0-5.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18395/shard-skl3/igt@kms_co...@pipe-a-ctm-0-5.html * igt@kms_cursor_crc@pipe-b-cursor-suspend: - shard-apl: [PASS][7] -> [DMESG-WARN][8] ([i915#1635] / [i915#180]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8921/shard-apl1/igt@kms_cursor_...@pipe-b-cursor-suspend.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18395/shard-apl1/igt@kms_cursor_...@pipe-b-cursor-suspend.html * igt@kms_draw_crc@draw-method-xrgb2101010-pwrite-ytiled: - shard-apl: [PASS][9] -> [DMESG-WARN][10] ([i915#1635] / [i915#1982]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8921/shard-apl4/igt@kms_draw_...@draw-method-xrgb2101010-pwrite-ytiled.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18395/shard-apl6/igt@kms_draw_...@draw-method-xrgb2101010-pwrite-ytiled.html * igt@kms_flip@dpms-vs-vblank-race@a-hdmi-a2: - shard-glk: [PASS][11] -> [FAIL][12] ([i915#407]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8921/shard-glk8/igt@kms_flip@dpms-vs-vblank-r...@a-hdmi-a2.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18395/shard-glk2/igt@kms_flip@dpms-vs-vblank-r...@a-hdmi-a2.html * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-blt: - shard-iclb: [PASS][13] -> [DMESG-WARN][14] ([i915#1982]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8921/shard-iclb8/igt@kms_frontbuffer_track...@fbc-1p-offscren-pri-shrfb-draw-blt.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18395/shard-iclb4/igt@kms_frontbuffer_track...@fbc-1p-offscren-pri-shrfb-draw-blt.html * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-shrfb-draw-pwrite: - shard-kbl: [PASS][15] -> [DMESG-WARN][16] ([i915#1982]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8921/shard-kbl4/igt@kms_frontbuffer_track...@fbc-1p-primscrn-pri-shrfb-draw-pwrite.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18395/shard-kbl4/igt@kms_frontbuffer_track...@fbc-1p-primscrn-pri-shrfb-draw-pwrite.html * igt@kms_frontbuffer_tracking@psr-1p-primscrn-indfb-plflip-blt: - shard-tglb: [PASS][17] -> [DMESG-WARN][18] ([i915#1982]) +2 similar issues [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8921/shard-tglb1/igt@kms_frontbuffer_track...@psr-1p-primscrn-indfb-plflip-blt.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18395/shard-tglb2/igt@kms_frontbuffer_track...@psr-1p-primscrn-indfb-plflip-blt.html * igt@kms_hdr@bpc-switch-dpms: - shard-skl: [PASS][19] -> [FAIL][20] ([i915#1188]) +1 similar issue [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8921/shard-skl6/igt@kms_...@bpc-switch-dpms.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18395/shard-skl4/igt@kms_...@bpc-switch-dpms.html * igt@kms_psr@psr2_suspend: - shard-iclb: [PASS][21] -> [SKIP][22] ([fdo#109441]) [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8921/shard-iclb2/igt@kms_psr@psr2_suspend.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18395/shard-iclb8/igt@kms_psr@psr2_suspend.html * igt@kms_setmode@basic: - shard-kbl: [PASS][23] -> [FAIL][24] ([i915#31]) [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8921/shard-kbl1/igt@kms_setm...@basic.html [24]: https://intel-gfx-ci.01.org/tree/drm-
Re: [Intel-gfx] [PATCH] iommu/intel: Handle 36b addressing for x86-32
Hi Chris,
On 2020/8/23 0:02, Chris Wilson wrote:
Beware that the address size for x86-32 may exceed unsigned long.
[0.368971] UBSAN: shift-out-of-bounds in drivers/iommu/intel/iommu.c:128:14
[0.369055] shift exponent 36 is too large for 32-bit type 'long unsigned
int'
If we don't handle the wide addresses, the pages are mismapped and the
device read/writes go astray, detected as DMAR faults and leading to
device failure. The behaviour changed (from working to broken) in commit
fa954e683178 ("iommu/vt-d: Delegate the dma domain to upper layer"), but
commit ("iommu/vt-d: Delegate the dma domain to upper layer")
and adjust the title as "iommu/vt-d: Handle 36bit addressing for x86-32"
with above two changes,
Acked-by: Lu Baolu
Best regards,
baolu
the error looks older.
Fixes: fa954e683178 ("iommu/vt-d: Delegate the dma domain to upper layer")
Signed-off-by: Chris Wilson
Cc: James Sewart
Cc: Lu Baolu
Cc: Joerg Roedel
Cc: # v5.3+
---
drivers/iommu/intel/iommu.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 2e9c8c3d0da4..ba78a2e854f9 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -123,29 +123,29 @@ static inline unsigned int level_to_offset_bits(int level)
return (level - 1) * LEVEL_STRIDE;
}
-static inline int pfn_level_offset(unsigned long pfn, int level)
+static inline int pfn_level_offset(u64 pfn, int level)
{
return (pfn >> level_to_offset_bits(level)) & LEVEL_MASK;
}
-static inline unsigned long level_mask(int level)
+static inline u64 level_mask(int level)
{
- return -1UL << level_to_offset_bits(level);
+ return -1ULL << level_to_offset_bits(level);
}
-static inline unsigned long level_size(int level)
+static inline u64 level_size(int level)
{
- return 1UL << level_to_offset_bits(level);
+ return 1ULL << level_to_offset_bits(level);
}
-static inline unsigned long align_to_level(unsigned long pfn, int level)
+static inline u64 align_to_level(u64 pfn, int level)
{
return (pfn + level_size(level) - 1) & level_mask(level);
}
static inline unsigned long lvl_to_nr_pages(unsigned int lvl)
{
- return 1 << min_t(int, (lvl - 1) * LEVEL_STRIDE, MAX_AGAW_PFN_WIDTH);
+ return 1UL << min_t(int, (lvl - 1) * LEVEL_STRIDE, MAX_AGAW_PFN_WIDTH);
}
/* VT-d pages must always be _smaller_ than MM pages. Otherwise things
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 00/29] treewide: Convert comma separated statements
There are many comma separated statements in the kernel. See:https://lore.kernel.org/lkml/alpine.DEB.2.22.394.2008201856110.2524@hadrien/ Convert the comma separated statements that are in if/do/while blocks to use braces and semicolons. Many comma separated statements still exist but those are changes for another day. Joe Perches (29): coding-style.rst: Avoid comma statements alpha: Avoid comma separated statements ia64: Avoid comma separated statements sparc: Avoid comma separated statements ata: Avoid comma separated statements drbd: Avoid comma separated statements lp: Avoid comma separated statements dma-buf: Avoid comma separated statements drm/gma500: Avoid comma separated statements drm/i915: Avoid comma separated statements hwmon: (scmi-hwmon): Avoid comma separated statements Input: MT - Avoid comma separated statements bcache: Avoid comma separated statements media: Avoid comma separated statements mtd: Avoid comma separated statements 8390: Avoid comma separated statements fs_enet: Avoid comma separated statements wan: sbni: Avoid comma separated statements s390/tty3270: Avoid comma separated statements scai/arm: Avoid comma separated statements media: atomisp: Avoid comma separated statements video: fbdev: Avoid comma separated statements fuse: Avoid comma separated statements reiserfs: Avoid comma separated statements lib/zlib: Avoid comma separated statements lib: zstd: Avoid comma separated statements ipv6: fib6: Avoid comma separated statements sunrpc: Avoid comma separated statements tools: Avoid comma separated statements Documentation/process/coding-style.rst| 17 + arch/alpha/kernel/pci_iommu.c | 8 +- arch/alpha/oprofile/op_model_ev4.c| 22 +- arch/alpha/oprofile/op_model_ev5.c| 8 +- arch/ia64/kernel/smpboot.c| 7 +- arch/sparc/kernel/smp_64.c| 7 +- drivers/ata/pata_icside.c | 21 +- drivers/block/drbd/drbd_receiver.c| 6 +- drivers/char/lp.c | 6 +- drivers/dma-buf/st-dma-fence.c| 7 +- drivers/gpu/drm/gma500/mdfld_intel_display.c | 44 ++- drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 8 +- drivers/gpu/drm/i915/gt/intel_gt_requests.c | 6 +- .../gpu/drm/i915/gt/selftest_workarounds.c| 6 +- drivers/gpu/drm/i915/intel_runtime_pm.c | 6 +- drivers/hwmon/scmi-hwmon.c| 6 +- drivers/input/input-mt.c | 11 +- drivers/md/bcache/bset.c | 12 +- drivers/md/bcache/sysfs.c | 6 +- drivers/media/i2c/msp3400-kthreads.c | 12 +- drivers/media/pci/bt8xx/bttv-cards.c | 6 +- drivers/media/pci/saa7134/saa7134-video.c | 7 +- drivers/mtd/devices/lart.c| 10 +- drivers/net/ethernet/8390/axnet_cs.c | 19 +- drivers/net/ethernet/8390/lib8390.c | 14 +- drivers/net/ethernet/8390/pcnet_cs.c | 6 +- .../ethernet/freescale/fs_enet/fs_enet-main.c | 11 +- drivers/net/wan/sbni.c| 101 +++--- drivers/s390/char/tty3270.c | 6 +- drivers/scsi/arm/cumana_2.c | 19 +- drivers/scsi/arm/eesox.c | 9 +- drivers/scsi/arm/powertec.c | 9 +- .../media/atomisp/pci/atomisp_subdev.c| 6 +- drivers/video/fbdev/tgafb.c | 12 +- fs/fuse/dir.c | 24 +- fs/reiserfs/fix_node.c| 36 ++- lib/zlib_deflate/deftree.c| 49 ++- lib/zstd/compress.c | 120 --- lib/zstd/fse_compress.c | 24 +- lib/zstd/huf_compress.c | 6 +- net/ipv6/ip6_fib.c| 12 +- net/sunrpc/sysctl.c | 6 +- tools/lib/subcmd/help.c | 10 +- tools/power/cpupower/utils/cpufreq-set.c | 14 +- tools/testing/selftests/vm/gup_benchmark.c| 18 +- tools/testing/selftests/vm/userfaultfd.c | 296 +++--- 46 files changed, 694 insertions(+), 382 deletions(-) -- 2.26.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 10/29] drm/i915: Avoid comma separated statements
Use semicolons and braces.
Signed-off-by: Joe Perches
---
drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 8 +---
drivers/gpu/drm/i915/gt/intel_gt_requests.c| 6 --
drivers/gpu/drm/i915/gt/selftest_workarounds.c | 6 --
drivers/gpu/drm/i915/intel_runtime_pm.c| 6 --
4 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
index 699125928272..114c13285ff1 100644
--- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
+++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
@@ -323,10 +323,12 @@ static int __gen8_ppgtt_alloc(struct i915_address_space *
const vm,
}
spin_lock(&pd->lock);
- if (likely(!pd->entry[idx]))
+ if (likely(!pd->entry[idx])) {
set_pd_entry(pd, idx, pt);
- else
- alloc = pt, pt = pd->entry[idx];
+ } else {
+ alloc = pt;
+ pt = pd->entry[idx];
+ }
}
if (lvl) {
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_requests.c
b/drivers/gpu/drm/i915/gt/intel_gt_requests.c
index 66fcbf9d0fdd..54408d0b5e6e 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_requests.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_requests.c
@@ -139,8 +139,10 @@ long intel_gt_retire_requests_timeout(struct intel_gt *gt,
long timeout)
LIST_HEAD(free);
interruptible = true;
- if (unlikely(timeout < 0))
- timeout = -timeout, interruptible = false;
+ if (unlikely(timeout < 0)) {
+ timeout = -timeout;
+ interruptible = false;
+ }
flush_submission(gt, timeout); /* kick the ksoftirqd tasklets */
spin_lock(&timelines->lock);
diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
index febc9e6692ba..3e4cbeed20bd 100644
--- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
@@ -521,8 +521,10 @@ static int check_dirty_whitelist(struct intel_context *ce)
srm = MI_STORE_REGISTER_MEM;
lrm = MI_LOAD_REGISTER_MEM;
- if (INTEL_GEN(engine->i915) >= 8)
- lrm++, srm++;
+ if (INTEL_GEN(engine->i915) >= 8) {
+ lrm++;
+ srm++;
+ }
pr_debug("%s: Writing garbage to %x\n",
engine->name, reg);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 153ca9e65382..f498f1c80755 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -201,8 +201,10 @@ __print_intel_runtime_pm_wakeref(struct drm_printer *p,
unsigned long rep;
rep = 1;
- while (i + 1 < dbg->count && dbg->owners[i + 1] == stack)
- rep++, i++;
+ while (i + 1 < dbg->count && dbg->owners[i + 1] == stack) {
+ rep++;
+ i++;
+ }
__print_depot_stack(stack, buf, PAGE_SIZE, 2);
drm_printf(p, "Wakeref x%lu taken at:\n%s", rep, buf);
}
--
2.26.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 3/3] drm/i915/display: Fix DRRS debugfs
On 2020-08-24 at 10:43:45 -0700, José Roberto de Souza wrote:
> Supported and enabled are different things so printing both.
>
> Cc: Anshuman Gupta
> Cc: Srinivas K
> Cc: Hariom Pandey
> Signed-off-by: José Roberto de Souza
> ---
> drivers/gpu/drm/i915/display/intel_display_debugfs.c | 12 ++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index f549381048b3..4b4cabf34d24 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -1069,10 +1069,18 @@ static void drrs_status_per_crtc(struct seq_file *m,
>
> drm_connector_list_iter_begin(dev, &conn_iter);
> drm_for_each_connector_iter(connector, &conn_iter) {
> + bool supported = false;
> +
> if (connector->state->crtc != &intel_crtc->base)
> continue;
>
> seq_printf(m, "%s:\n", connector->name);
> +
> + if (connector->connector_type == DRM_MODE_CONNECTOR_eDP &&
> + dev_priv->vbt.drrs_type == SEAMLESS_DRRS_SUPPORT)
IMO use dev_priv->drrs.type here, becuase DRRS is not supported in case
intel_panel_edid_downclock_mode() return a NULL downclock mode in
intel_dp_drrs_init().
Thanks,
Anshuman Gupta
> + supported = true;
> +
> + seq_printf(m, "\tDRRS Supported: %s\n", yesno(supported));
> }
> drm_connector_list_iter_end(&conn_iter);
>
> @@ -1083,7 +1091,7 @@ static void drrs_status_per_crtc(struct seq_file *m,
>
> mutex_lock(&drrs->mutex);
> /* DRRS Supported */
> - seq_puts(m, "\tDRRS Supported: Yes\n");
> + seq_puts(m, "\tDRRS Enabled: Yes\n");
>
> /* disable_drrs() will make drrs->dp NULL */
> if (!drrs->dp) {
> @@ -1118,7 +1126,7 @@ static void drrs_status_per_crtc(struct seq_file *m,
> mutex_unlock(&drrs->mutex);
> } else {
> /* DRRS not supported. Print the VBT parameter*/
> - seq_puts(m, "\tDRRS Supported : No");
> + seq_puts(m, "\tDRRS Enabled : No");
> }
> seq_puts(m, "\n");
> }
> --
> 2.28.0
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
