[Intel-gfx] ✓ Fi.CI.IGT: success for Improve crc-core driver interface (rev13)
== Series Details == Series: Improve crc-core driver interface (rev13) URL : https://patchwork.freedesktop.org/series/45246/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4692_full -> Patchwork_9980_full = == Summary - SUCCESS == No regressions found. == Known issues == Here are the changes found in Patchwork_9980_full that come from known issues: === IGT changes === Issues hit igt@gem_exec_await@wide-contexts: shard-apl: PASS -> FAIL (fdo#106680, fdo#105900) igt@gem_ppgtt@blt-vs-render-ctxn: shard-kbl: PASS -> INCOMPLETE (fdo#106023, fdo#103665) igt@gem_workarounds@suspend-resume: shard-glk: PASS -> FAIL (fdo#103375) igt@kms_setmode@basic: shard-kbl: PASS -> FAIL (fdo#99912) Possible fixes igt@drv_suspend@shrink: shard-glk: FAIL (fdo#106886) -> PASS igt@kms_setmode@basic: shard-apl: FAIL (fdo#99912) -> PASS fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375 fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 fdo#105900 https://bugs.freedesktop.org/show_bug.cgi?id=105900 fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023 fdo#106680 https://bugs.freedesktop.org/show_bug.cgi?id=106680 fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 == Participating hosts (5 -> 5) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4692 -> Patchwork_9980 CI_DRM_4692: d53f119472fc7daa532e46ea77098e9e9db2ac10 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4607: 6e0b3e7a2d241af36f8c6b1cc335aa1db3532d29 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9980: 70f9435337d30e26426ebaa311efb29f7d52051a @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9980/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for Improve crc-core driver interface (rev13)
== Series Details == Series: Improve crc-core driver interface (rev13) URL : https://patchwork.freedesktop.org/series/45246/ State : failure == Summary == = CI Bug Log - changes from CI_DRM_4692 -> Patchwork_9981 = == Summary - FAILURE == Serious unknown changes coming with Patchwork_9981 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_9981, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/45246/revisions/13/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_9981: === IGT changes === Possible regressions igt@drv_selftest@live_hangcheck: fi-cfl-guc: PASS -> DMESG-FAIL == Known issues == Here are the changes found in Patchwork_9981 that come from known issues: === IGT changes === Issues hit igt@drv_selftest@live_guc: fi-skl-guc: NOTRUN -> DMESG-WARN (fdo#107258, fdo#107175) igt@drv_selftest@live_hangcheck: fi-bdw-5557u: PASS -> DMESG-FAIL (fdo#106560) igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b: fi-snb-2520m: PASS -> INCOMPLETE (fdo#103713) fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713 fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560 fdo#107175 https://bugs.freedesktop.org/show_bug.cgi?id=107175 fdo#107258 https://bugs.freedesktop.org/show_bug.cgi?id=107258 == Participating hosts (51 -> 48) == Additional (1): fi-skl-guc Missing(4): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u == Build changes == * Linux: CI_DRM_4692 -> Patchwork_9981 CI_DRM_4692: d53f119472fc7daa532e46ea77098e9e9db2ac10 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4607: 6e0b3e7a2d241af36f8c6b1cc335aa1db3532d29 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9981: 16455237058ae7e10b0fdee464c5e49e5164df75 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 16455237058a Revert "drm: crc: Wait for a frame before returning from open()" 7a546236475c drm/crc: Cleanup crtc_crc_open function 1fa679413c80 drm/vkms/crc: Implement verify_crc_source callback == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9981/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for Improve crc-core driver interface (rev13)
== Series Details ==
Series: Improve crc-core driver interface (rev13)
URL : https://patchwork.freedesktop.org/series/45246/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4692 -> Patchwork_9980 =
== Summary - SUCCESS ==
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/45246/revisions/13/mbox/
== Known issues ==
Here are the changes found in Patchwork_9980 that come from known issues:
=== IGT changes ===
Issues hit
igt@drv_selftest@live_hangcheck:
fi-kbl-7560u: PASS -> DMESG-FAIL (fdo#106560, fdo#106947)
igt@kms_frontbuffer_tracking@basic:
fi-hsw-peppy: PASS -> DMESG-FAIL (fdo#102614)
{fi-byt-clapper}: PASS -> FAIL (fdo#103167)
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
== Participating hosts (51 -> 47) ==
Missing(4): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4692 -> Patchwork_9980
CI_DRM_4692: d53f119472fc7daa532e46ea77098e9e9db2ac10 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4607: 6e0b3e7a2d241af36f8c6b1cc335aa1db3532d29 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9980: 70f9435337d30e26426ebaa311efb29f7d52051a @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
70f9435337d3 Revert "drm: crc: Wait for a frame before returning from open()"
314b22dfdb36 drm/crc: Cleanup crtc_crc_open function
b30e97d93e8d drm/vkms/crc: Implement verify_crc_source callback
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9980/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH V3 1/3] drm/vkms/crc: Implement verify_crc_source callback
This patch implements "verify_crc_source" callback function for
Virtual KMS drm driver.
Changes Since V1:
- update values_cnt in verify_crc_source
Changes Since V2:
- don't return early from set_crc_source to keep behavior same (Haneen)
Cc: Haneen Mohammed
Signed-off-by: Mahesh Kumar
---
drivers/gpu/drm/vkms/vkms_crc.c | 36
drivers/gpu/drm/vkms/vkms_crtc.c | 1 +
drivers/gpu/drm/vkms/vkms_drv.h | 2 ++
3 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_crc.c b/drivers/gpu/drm/vkms/vkms_crc.c
index 37d717f38e3c..68421d3d809a 100644
--- a/drivers/gpu/drm/vkms/vkms_crc.c
+++ b/drivers/gpu/drm/vkms/vkms_crc.c
@@ -70,6 +70,37 @@ void vkms_crc_work_handle(struct work_struct *work)
drm_crtc_add_crc_entry(crtc, true, crtc_state->n_frame, );
}
+static int vkms_crc_parse_source(const char *src_name, bool *enabled)
+{
+ int ret = 0;
+
+ if (!src_name) {
+ *enabled = false;
+ } else if (strcmp(src_name, "auto") == 0) {
+ *enabled = true;
+ } else {
+ *enabled = false;
+ ret = -EINVAL;
+ }
+
+ return ret;
+}
+
+int vkms_verify_crc_source(struct drm_crtc *crtc, const char *src_name,
+ size_t *values_cnt)
+{
+ bool enabled;
+
+ if (vkms_crc_parse_source(src_name, ) < 0) {
+ DRM_DEBUG_DRIVER("unknown source %s\n", src_name);
+ return -EINVAL;
+ }
+
+ *values_cnt = 1;
+
+ return 0;
+}
+
int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
size_t *values_cnt)
{
@@ -78,10 +109,7 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const char
*src_name,
unsigned long flags;
int ret = 0;
- if (src_name && strcmp(src_name, "auto") == 0)
- enabled = true;
- else if (src_name)
- ret = -EINVAL;
+ ret = vkms_crc_parse_source(src_name, );
*values_cnt = 1;
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index bfe6e0312cc4..9d0b1a325a78 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -140,6 +140,7 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
.enable_vblank = vkms_enable_vblank,
.disable_vblank = vkms_disable_vblank,
.set_crc_source = vkms_set_crc_source,
+ .verify_crc_source = vkms_verify_crc_source,
};
static void vkms_crtc_atomic_enable(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index f156c930366a..090c5e4f5544 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -125,6 +125,8 @@ void vkms_gem_vunmap(struct drm_gem_object *obj);
/* CRC Support */
int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
size_t *values_cnt);
+int vkms_verify_crc_source(struct drm_crtc *crtc, const char *source_name,
+ size_t *values_cnt);
void vkms_crc_work_handle(struct work_struct *work);
#endif /* _VKMS_DRV_H_ */
--
2.16.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH V2 1/3] drm/vkms/crc: Implement verify_crc_source callback
Hi Haneen,
On 8/21/2018 4:09 AM, Haneen Mohammed wrote:
On Tue, Aug 14, 2018 at 08:31:03AM +0530, Mahesh Kumar wrote:
This patch implements "verify_crc_source" callback function for
Virtual KMS drm driver.
Changes Since V1:
- update values_cnt in verify_crc_source
Cc: Haneen Mohammed
Signed-off-by: Mahesh Kumar
---
drivers/gpu/drm/vkms/vkms_crc.c | 38 ++
drivers/gpu/drm/vkms/vkms_crtc.c | 1 +
drivers/gpu/drm/vkms/vkms_drv.h | 2 ++
3 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_crc.c b/drivers/gpu/drm/vkms/vkms_crc.c
index 37d717f38e3c..b2a484b4e2ad 100644
--- a/drivers/gpu/drm/vkms/vkms_crc.c
+++ b/drivers/gpu/drm/vkms/vkms_crc.c
@@ -70,6 +70,37 @@ void vkms_crc_work_handle(struct work_struct *work)
drm_crtc_add_crc_entry(crtc, true, crtc_state->n_frame, );
}
+static int vkms_crc_parse_source(const char *src_name, bool *enabled)
+{
+ int ret = 0;
+
+ if (!src_name) {
+ *enabled = false;
+ } else if (strcmp(src_name, "auto") == 0) {
+ *enabled = true;
+ } else {
+ *enabled = false;
+ ret = -EINVAL;
+ }
+
+ return ret;
+}
+
+int vkms_verify_crc_source(struct drm_crtc *crtc, const char *src_name,
+ size_t *values_cnt)
+{
+ bool enabled;
+
+ if (vkms_crc_parse_source(src_name, ) < 0) {
+ DRM_DEBUG_DRIVER("unknown source %s\n", src_name);
+ return -EINVAL;
+ }
+
+ *values_cnt = 1;
+
+ return 0;
+}
+
int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
size_t *values_cnt)
{
@@ -78,10 +109,9 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const char
*src_name,
unsigned long flags;
int ret = 0;
- if (src_name && strcmp(src_name, "auto") == 0)
- enabled = true;
- else if (src_name)
- ret = -EINVAL;
+ ret = vkms_crc_parse_source(src_name, );
+ if (ret)
+ return ret;
I think the return value after vkms_crc_parse_source should be called
once instead at the end of vkms_set_crc_source otherwise crc_enabled
won't be updated?
Here I'm assuming if src_name is wrong we don't need to proceed further,
let the CRC generation to continue whatever it was doing (enabled or
disabled).
But anyway that is changing the original design, will remove above
return to keep the behavior same.
-Mahesh
- Haneen
*values_cnt = 1;
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index bfe6e0312cc4..9d0b1a325a78 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -140,6 +140,7 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
.enable_vblank = vkms_enable_vblank,
.disable_vblank = vkms_disable_vblank,
.set_crc_source = vkms_set_crc_source,
+ .verify_crc_source = vkms_verify_crc_source,
};
static void vkms_crtc_atomic_enable(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index f156c930366a..090c5e4f5544 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -125,6 +125,8 @@ void vkms_gem_vunmap(struct drm_gem_object *obj);
/* CRC Support */
int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
size_t *values_cnt);
+int vkms_verify_crc_source(struct drm_crtc *crtc, const char *source_name,
+ size_t *values_cnt);
void vkms_crc_work_handle(struct work_struct *work);
#endif /* _VKMS_DRV_H_ */
--
2.16.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/psr: Add PSR mode/revision to debugfs (rev3)
== Series Details == Series: drm/i915/psr: Add PSR mode/revision to debugfs (rev3) URL : https://patchwork.freedesktop.org/series/47902/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4691_full -> Patchwork_9979_full = == Summary - WARNING == Minor unknown changes coming with Patchwork_9979_full need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_9979_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_9979_full: === IGT changes === Warnings igt@pm_rc6_residency@rc6-accuracy: shard-kbl: PASS -> SKIP == Known issues == Here are the changes found in Patchwork_9979_full that come from known issues: === IGT changes === Issues hit igt@kms_cursor_crc@cursor-128x128-suspend: shard-kbl: PASS -> DMESG-WARN (fdo#103313) igt@kms_flip@2x-flip-vs-expired-vblank: shard-glk: PASS -> FAIL (fdo#105363) igt@kms_setmode@basic: shard-kbl: PASS -> FAIL (fdo#99912) Possible fixes igt@drv_suspend@shrink: shard-glk: FAIL (fdo#106886) -> PASS igt@gem_workarounds@suspend-resume: shard-glk: FAIL (fdo#103375) -> PASS fdo#103313 https://bugs.freedesktop.org/show_bug.cgi?id=103313 fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375 fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363 fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 == Participating hosts (5 -> 5) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4691 -> Patchwork_9979 CI_DRM_4691: 2d75266982a5dae956c10e683a1a74d977d88e09 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4607: 6e0b3e7a2d241af36f8c6b1cc335aa1db3532d29 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9979: 3b646742f6a55774f7e1ffd8ccaba13ade2a08c3 @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9979/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/5] drm/i915: kill intel_display_power_well_is_enabled()
== Series Details == Series: series starting with [1/5] drm/i915: kill intel_display_power_well_is_enabled() URL : https://patchwork.freedesktop.org/series/48466/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4691_full -> Patchwork_9978_full = == Summary - SUCCESS == No regressions found. == Known issues == Here are the changes found in Patchwork_9978_full that come from known issues: === IGT changes === Issues hit igt@gem_ppgtt@blt-vs-render-ctxn: shard-kbl: PASS -> INCOMPLETE (fdo#103665, fdo#106023) igt@kms_fbcon_fbt@fbc-suspend: shard-kbl: PASS -> DMESG-WARN (fdo#103313) igt@kms_setmode@basic: shard-kbl: PASS -> FAIL (fdo#99912) Possible fixes igt@drv_suspend@shrink: shard-glk: FAIL (fdo#106886) -> PASS igt@kms_setmode@basic: shard-apl: FAIL (fdo#99912) -> PASS fdo#103313 https://bugs.freedesktop.org/show_bug.cgi?id=103313 fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023 fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 == Participating hosts (5 -> 5) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4691 -> Patchwork_9978 CI_DRM_4691: 2d75266982a5dae956c10e683a1a74d977d88e09 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4607: 6e0b3e7a2d241af36f8c6b1cc335aa1db3532d29 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9978: 5e7669b1dc9b95218d7deb4709a83ed1905b54b3 @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9978/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915/psr: Remove wait_for_idle() for PSR2
On Wed, Aug 15, 2018 at 06:35:15PM -0700, Dhinakaran Pandiyan wrote:
> On Mon, 2018-08-13 at 18:10 +, Pandiyan, Dhinakaran wrote:
> > On Mon, 2018-08-13 at 09:57 -0700, Rodrigo Vivi wrote:
> > > On Thu, Aug 09, 2018 at 05:41:35PM -0700, Dhinakaran Pandiyan
> > > wrote:
> > > > CI runs show PSR2 does not go to IDLE with selective update
> > > > enabled
> > > > on
> > > > all PSR exit triggers. Specifically, logs indicate the hardware
> > > > enters
> > > > "SLEEP Selective Update" and not "IDLE Reset state' like the
> > > > kernel
> > > > expects. This check was added for PSR1 but incorrectly extended
> > > > to
> > > > PSR2,
> > > > remove this check for PSR2 as there is a plan to test only PSR1
> > > > on
> > > > PSR2
> > > > panels.
> > > >
> > > > Also add bspec reference to the comment about idle timeout.
> > > >
> > > > Cc: Tarun Vyas
> > > > Cc: José Roberto de Souza
> > > > Cc: Rodrigo Vivi
> > > > Signed-off-by: Dhinakaran Pandiyan > > > >
> > > > ---
> > > > drivers/gpu/drm/i915/intel_psr.c | 39 --
> > > > --
> > > > 1 file changed, 14 insertions(+), 25 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > > b/drivers/gpu/drm/i915/intel_psr.c
> > > > index 5686ddaa6a72..09be9bfee2be 100644
> > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > @@ -722,37 +722,26 @@ int intel_psr_wait_for_idle(const struct
> > > > intel_crtc_state *new_crtc_state,
> > > > {
> > > > struct intel_crtc *crtc = to_intel_crtc(new_crtc_state-
> > > > > base.crtc);
> > > >
> > > > struct drm_i915_private *dev_priv = to_i915(crtc-
> > > > > base.dev);
> > > >
> > > > - i915_reg_t reg;
> > > > - u32 mask;
> > > > -
> > > > - if (!new_crtc_state->has_psr)
> > > > - return 0;
> > > >
> > > > /*
> > > > -* The sole user right now is intel_pipe_update_start(),
> > > > -* which won't race with psr_enable/disable, which is
> > > > -* where psr2_enabled is written to. So, we don't need
> > > > -* to acquire the psr.lock. More importantly, we want
> > > > the
> > > > -* latency inside intel_pipe_update_start() to be as low
> > > > -* as possible, so no need to acquire psr.lock when it
> > > > is
> > > > -* not needed and will induce latencies in the atomic
> > > > -* update path.
> > > > +* The sole user right now is intel_pipe_update_start(),
> > > > which won't
> > > > +* race with psr_enable/disable where psr2_enabled is
> > > > written to. So, we
> > > > +* don't need to acquire the psr.lock. More importantly,
> > > > we want the
> > > > +* latency inside intel_pipe_update_start() to be as low
> > > > as possible, so
> > > > +* no need to acquire psr.lock when it is not needed and
> > > > will induce
> > > > +* latencies in the atomic update path.
> > > > */
> > >
> > > I think we shouldn't change this format here to keep patch
> > > cleaner...
> > > if there is any change here I couldn't see because it is changing
> > > all
> > > lines and if there is no change I think it is better not to touch
> > > because
> > > it removes the focus of the real changes.
> >
> > Okay.
> > >
> > > > - if (dev_priv->psr.psr2_enabled) {
> > > > - reg = EDP_PSR2_STATUS;
> > > > - mask = EDP_PSR2_STATUS_STATE_MASK;
> > > > - } else {
> > > > - reg = EDP_PSR_STATUS;
> > > > - mask = EDP_PSR_STATUS_STATE_MASK;
> > > > - }
> > > > + if (!new_crtc_state->has_psr || READ_ONCE(dev_priv-
> > > > > psr.psr2_enabled))
> > >
> > > I now see that we are removing psr2 of the picture, but I don't see
> > > how we are
> > > improving psr2 situation here.
> > > what am I missing?
> > >
> >
> > When the patch was written, we did not have sufficient tests to tell
> > us
> > the wait_for_idle condition was wrong for PSR2. It was not known
> > whether the wait was *necessary* for PSR2, think of this as a partial
> > revert. Now that CI has pointed out, (and I checked with a PSR2
> > panel)
> > that the condition is wrong, we should be removing it for PSR2. If
> > you
> > think about it, it does improve PSR2 my removing irrelevant code.
> >
> I'll submit a new version without the comment diff if you are satisfied
> with the above reasoning.
Please add a "FIXME:" here and we can fix psr2 situation later when we
understand
it better...
so we at least unblock PSR for now...
>
> >
> > > > + return 0;
> > > >
> > > > /*
> > > > -* Max time for PSR to idle = Inverse of the refresh
> > > > rate
> > > > +
> > > > -* 6 ms of exit training time + 1.5 ms of aux channel
> > > > -* handshake. 50 msec is defesive enough to cover
> > > > everything.
> > > > +* From Bspec Panel Self Refresh (BDW+):
> > >
> > > This is another case, if we didn't
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/psr: Add PSR mode/revision to debugfs (rev3)
== Series Details ==
Series: drm/i915/psr: Add PSR mode/revision to debugfs (rev3)
URL : https://patchwork.freedesktop.org/series/47902/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4691 -> Patchwork_9979 =
== Summary - SUCCESS ==
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/47902/revisions/3/mbox/
== Known issues ==
Here are the changes found in Patchwork_9979 that come from known issues:
=== IGT changes ===
Issues hit
igt@gem_exec_suspend@basic-s4-devices:
fi-kbl-7500u: PASS -> DMESG-WARN (fdo#105128, fdo#107139)
Possible fixes
igt@drv_selftest@live_hangcheck:
fi-skl-guc: DMESG-FAIL (fdo#107174) -> PASS
fi-kbl-guc: DMESG-FAIL (fdo#106947) -> PASS
fi-bxt-dsi: DMESG-FAIL (fdo#106560) -> PASS
{igt@pm_rpm@module-reload}:
fi-cnl-psr: WARN (fdo#107602) -> PASS
Warnings
{igt@kms_psr@primary_page_flip}:
fi-cnl-psr: DMESG-WARN (fdo#107372) -> DMESG-FAIL (fdo#107372)
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139
fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174
fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372
fdo#107602 https://bugs.freedesktop.org/show_bug.cgi?id=107602
== Participating hosts (51 -> 48) ==
Additional (1): fi-snb-2520m
Missing(4): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4691 -> Patchwork_9979
CI_DRM_4691: 2d75266982a5dae956c10e683a1a74d977d88e09 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4607: 6e0b3e7a2d241af36f8c6b1cc335aa1db3532d29 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9979: 3b646742f6a55774f7e1ffd8ccaba13ade2a08c3 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
3b646742f6a5 drm/i915/psr: Add PSR mode/revision to debugfs
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9979/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Show debugfs dpcd read failure directly
On Mon, Aug 20, 2018 at 12:10:08PM -0700, Pandiyan, Dhinakaran wrote:
>
>
> On Mon, 2018-08-20 at 10:15 +0300, Jani Nikula wrote:
> > On Fri, 20 Jul 2018, Rodrigo Vivi wrote:
> > > Instead of using a backchannel if some dpcd read failed we
> > > can show that directly on debugfs output.
> > >
> > > We are not returning an error because we might still want
> > > to know if sub-sequent reads works, but we shouldn't
> > > need to check 2 places to see why reg is not on the list.
> >
> > Should we just nuke this debugfs and use the aux chardev interface to
> > dpcd instead?
>
> Given that this debugfs does not print decoded information, I think, it
> offers very little benefit over direct reads. We also print some DPCD's
> in dmesg, so I'm in favour of killing it. Don't see the file being used
> in IGTs either.
>
> Your comment also reminds about the IGT tool that Tarun started writing
> for reading DPCD.
There might be people out there using this for debug. So we should first
be able to do this in some place like IGT:
https://patchwork.freedesktop.org/series/48470/ ?
then we remove. But that discussion might take a while. So I'd like
to move with this fix here now and remove dpcd_show completely when
we create some infrastructure on IGT.
>
> -DK
>
>
> >
> > BR,
> > Jani.
> >
> > >
> > > Cc: Jani Nikula
> > > Cc: Dhinakaran Pandiyan
> > > Signed-off-by: Rodrigo Vivi
> > > ---
> > > drivers/gpu/drm/i915/i915_debugfs.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > > b/drivers/gpu/drm/i915/i915_debugfs.c
> > > index 59dc0610ea44..5d8da4e8c444 100644
> > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > @@ -4846,8 +4846,8 @@ static int i915_dpcd_show(struct seq_file *m,
> > > void *data)
> > >
> > > err = drm_dp_dpcd_read(_dp->aux, b->offset,
> > > buf, size);
> > > if (err <= 0) {
> > > - DRM_ERROR("dpcd read (%zu bytes at %u)
> > > failed (%zd)\n",
> > > - size, b->offset, err);
> > > + seq_printf(m, "dpcd read (%zu bytes at %u)
> > > failed (%zd)\n",
> > > +size, b->offset, err);
> > > continue;
> > > }
> >
> >
___
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] drm/i915: kill intel_display_power_well_is_enabled()
== Series Details ==
Series: series starting with [1/5] drm/i915: kill
intel_display_power_well_is_enabled()
URL : https://patchwork.freedesktop.org/series/48466/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4691 -> Patchwork_9978 =
== Summary - SUCCESS ==
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/48466/revisions/1/mbox/
== Known issues ==
Here are the changes found in Patchwork_9978 that come from known issues:
=== IGT changes ===
Issues hit
igt@drv_selftest@live_coherency:
fi-gdg-551: PASS -> DMESG-FAIL (fdo#107164)
igt@drv_selftest@live_hangcheck:
fi-bxt-j4205: PASS -> DMESG-FAIL (fdo#106560)
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
{fi-byt-clapper}: PASS -> FAIL (fdo#107362)
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
{fi-byt-clapper}: PASS -> FAIL (fdo#103191, fdo#107362)
Possible fixes
igt@drv_selftest@live_hangcheck:
fi-skl-guc: DMESG-FAIL (fdo#107174) -> PASS
fi-kbl-guc: DMESG-FAIL (fdo#106947) -> PASS
fi-bxt-dsi: DMESG-FAIL (fdo#106560) -> PASS
{igt@kms_psr@primary_mmap_gtt}:
fi-cnl-psr: DMESG-WARN (fdo#107372) -> PASS
{igt@pm_rpm@module-reload}:
fi-cnl-psr: WARN (fdo#107602) -> PASS
Warnings
{igt@kms_psr@primary_page_flip}:
fi-cnl-psr: DMESG-WARN (fdo#107372) -> DMESG-FAIL (fdo#107372)
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
fdo#107164 https://bugs.freedesktop.org/show_bug.cgi?id=107164
fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372
fdo#107602 https://bugs.freedesktop.org/show_bug.cgi?id=107602
== Participating hosts (51 -> 48) ==
Additional (1): fi-snb-2520m
Missing(4): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4691 -> Patchwork_9978
CI_DRM_4691: 2d75266982a5dae956c10e683a1a74d977d88e09 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4607: 6e0b3e7a2d241af36f8c6b1cc335aa1db3532d29 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9978: 5e7669b1dc9b95218d7deb4709a83ed1905b54b3 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
5e7669b1dc9b drm/i915: use the SW-based pw->hw_enabled check instead of reading
registers
49dc6855112d drm/i915: move lookup_power_well() up
7d5567ea0755 drm/i915: use for_each_power_well in lookup_power_well()
c740d932780f drm/i915: WARN() if we can't lookup_power_well()
20b826691fa5 drm/i915: kill intel_display_power_well_is_enabled()
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9978/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] Skip VBlank tests in modules without VBlank
The kms_flip test does not support drivers without VBlank which exclude
some virtual drivers. This patch adds a function that checks if a module
has a VBlank or not; if a module has VBlank than kms_flip will execute
all the VBlank tests, otherwise, VBlank tests will be skipped.
Signed-off-by: Rodrigo Siqueira
---
lib/igt_aux.c| 14 ++
lib/igt_aux.h| 2 ++
tests/kms_flip.c | 26 --
3 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 1250d5c5..da5be4bb 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -564,6 +564,20 @@ bool igt_aub_dump_enabled(void)
return dump_aub;
}
+bool igt_there_is_vblank(int drm_fd)
+{
+ drmVBlank dummy_vbl;
+ int ret;
+
+ dummy_vbl.request.type = DRM_VBLANK_ABSOLUTE;
+ ret = drmWaitVBlank(drm_fd, _vbl);
+
+ if (ret < 0)
+ return false;
+
+ return true;
+}
+
/* other helpers */
/**
* igt_exchange_int:
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index ef89faa9..933055e8 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -119,6 +119,8 @@ bool igt_check_boolean_env_var(const char *env_var, bool
default_value);
bool igt_aub_dump_enabled(void);
+bool igt_there_is_vblank(int fd);
+
/* suspend/hibernate and auto-resume system */
/**
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index 393d690a..770fa5f7 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -73,6 +73,7 @@
#define TEST_TS_CONT (1 << 27)
#define TEST_BO_TOOBIG (1 << 28)
+#define TEST_NO_VBLANK (1 << 29)
#define TEST_BASIC (1 << 30)
#define EVENT_FLIP (1 << 0)
@@ -125,6 +126,18 @@ struct event_state {
int seq_step;
};
+static bool vblank_dependence(int flags)
+{
+ int vblank_flags = TEST_VBLANK | TEST_VBLANK_BLOCK |
+ TEST_VBLANK_ABSOLUTE | TEST_VBLANK_EXPIRED_SEQ |
+ TEST_TS_CONT | TEST_CHECK_TS | TEST_VBLANK_RACE;
+
+ if (flags & vblank_flags)
+ return true;
+
+ return false;
+}
+
static float timeval_float(const struct timeval *tv)
{
return tv->tv_sec + tv->tv_usec / 100.0f;
@@ -493,11 +506,11 @@ static void check_state(const struct test_output *o,
const struct event_state *e
/* check only valid if no modeset happens in between, that increments by
* (1 << 23) on each step. This bounding matches the one in
* DRM_IOCTL_WAIT_VBLANK. */
- if (!(o->flags & (TEST_DPMS | TEST_MODESET)))
+ if (!(o->flags & (TEST_DPMS | TEST_MODESET | TEST_NO_VBLANK))) {
igt_assert_f(es->current_seq - (es->last_seq + o->seq_step) <=
1UL << 23,
"unexpected %s seq %u, should be >= %u\n",
es->name, es->current_seq, es->last_seq +
o->seq_step);
-
+ }
/* Check that the vblank frame didn't wrap unexpectedly. */
if (o->flags & TEST_TS_CONT) {
/* Ignore seq_step here since vblank waits time out immediately
@@ -1204,6 +1217,7 @@ static void run_test_on_crtc_set(struct test_output *o,
int *crtc_idxs,
unsigned bo_size = 0;
uint64_t tiling;
int i;
+ bool vblank = true;
switch (crtc_count) {
case 1:
@@ -1297,6 +1311,14 @@ static void run_test_on_crtc_set(struct test_output *o,
int *crtc_idxs,
}
igt_assert(fb_is_bound(o, o->fb_ids[0]));
+ vblank = igt_there_is_vblank(drm_fd);
+ if (!vblank) {
+ if (vblank_dependence(o->flags))
+ igt_require_f(vblank, "There is no Vblank\n");
+ else
+ o->flags |= TEST_NO_VBLANK;
+ }
+
/* quiescent the hw a bit so ensure we don't miss a single frame */
if (o->flags & TEST_CHECK_TS)
calibrate_ts(o, crtc_idxs[0]);
--
2.18.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3] drm/i915/psr: Add PSR mode/revision to debugfs
Log the PSR mode/revision (PSR1 or PSR2) in the debugfs file i915_edp_psr_status. Signed-off-by: Azhar Shaikh --- Changes in v3: - rebased Changes in v2: - Fix checkpatch warning. - Add Reviewed-by: from v1 drivers/gpu/drm/i915/i915_debugfs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 26b7e5276b15..bb94ed33a7da 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2708,7 +2708,9 @@ static int i915_edp_psr_status(struct seq_file *m, void *data) intel_runtime_pm_get(dev_priv); mutex_lock(_priv->psr.lock); - seq_printf(m, "Enabled: %s\n", yesno(dev_priv->psr.enabled)); + seq_printf(m, "PSR mode: %s\n", + dev_priv->psr.psr2_enabled ? "PSR2" : "PSR1"); + seq_printf(m, "Enabled: %s\n", yesno((bool)dev_priv->psr.enabled)); seq_printf(m, "Busy frontbuffer bits: 0x%03x\n", dev_priv->psr.busy_frontbuffer_bits); -- 1.9.1 ___ 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] drm/i915: kill intel_display_power_well_is_enabled()
== Series Details == Series: series starting with [1/5] drm/i915: kill intel_display_power_well_is_enabled() URL : https://patchwork.freedesktop.org/series/48466/ State : warning == Summary == $ dim checkpatch origin/drm-tip 20b826691fa5 drm/i915: kill intel_display_power_well_is_enabled() c740d932780f drm/i915: WARN() if we can't lookup_power_well() 7d5567ea0755 drm/i915: use for_each_power_well in lookup_power_well() 49dc6855112d drm/i915: move lookup_power_well() up 5e7669b1dc9b drm/i915: use the SW-based pw->hw_enabled check instead of reading registers -:15: WARNING:BAD_SIGN_OFF: Non-standard signature: Requested-by: #15: Requested-by: José Roberto de Souza total: 0 errors, 1 warnings, 0 checks, 10 lines checked ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/5] drm/i915: use the SW-based pw->hw_enabled check instead of reading registers
I can't find a reason why we would want to call is_enabled(), which
does a register read, instead of just relying on our tracking with
hw_enabled. Let's try to trust our hardware sync.
Cc: Imre Deak
Requested-by: José Roberto de Souza
Signed-off-by: Paulo Zanoni
---
drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index f38a049861e6..76bb2e06fef1 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -670,9 +670,8 @@ static void assert_can_enable_dc5(struct drm_i915_private
*dev_priv)
{
struct i915_power_well *pg2 = lookup_power_well(dev_priv,
SKL_DISP_PW_2);
- bool pg2_enabled = pg2->desc->ops->is_enabled(dev_priv, pg2);
- WARN_ONCE(pg2_enabled, "PG2 not disabled to enable DC5.\n");
+ WARN_ONCE(pg2->hw_enabled, "PG2 not disabled to enable DC5.\n");
WARN_ONCE((I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5),
"DC5 already programmed to be enabled.\n");
--
2.14.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/5] drm/i915: move lookup_power_well() up
There's no need for that forward declaration.
Cc: Imre Deak
Signed-off-by: Paulo Zanoni
---
drivers/gpu/drm/i915/intel_runtime_pm.c | 46 +++--
1 file changed, 21 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 6f5a2f00a12d..f38a049861e6 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -49,10 +49,6 @@
* present for a given platform.
*/
-static struct i915_power_well *
-lookup_power_well(struct drm_i915_private *dev_priv,
- enum i915_power_well_id power_well_id);
-
const char *
intel_display_power_domain_str(enum intel_display_power_domain domain)
{
@@ -649,6 +645,27 @@ static void assert_csr_loaded(struct drm_i915_private
*dev_priv)
WARN_ONCE(!I915_READ(CSR_HTP_SKL), "CSR HTP Not fine\n");
}
+static struct i915_power_well *
+lookup_power_well(struct drm_i915_private *dev_priv,
+ enum i915_power_well_id power_well_id)
+{
+ struct i915_power_well *power_well;
+
+ for_each_power_well(dev_priv, power_well)
+ if (power_well->desc->id == power_well_id)
+ return power_well;
+
+ /*
+* It's not feasible to add error checking code to the callers since
+* this condition really shouldn't happen and it doesn't even make sense
+* to abort things like display initialization sequences. Just return
+* the first power well and hope the WARN gets reported so we can fix
+* our driver.
+*/
+ WARN(1, "Power well %d not defined for this platform\n", power_well_id);
+ return _priv->power_domains.power_wells[0];
+}
+
static void assert_can_enable_dc5(struct drm_i915_private *dev_priv)
{
struct i915_power_well *pg2 = lookup_power_well(dev_priv,
@@ -1081,27 +1098,6 @@ static void vlv_dpio_cmn_power_well_disable(struct
drm_i915_private *dev_priv,
#define POWER_DOMAIN_MASK (GENMASK_ULL(POWER_DOMAIN_NUM - 1, 0))
-static struct i915_power_well *
-lookup_power_well(struct drm_i915_private *dev_priv,
- enum i915_power_well_id power_well_id)
-{
- struct i915_power_well *power_well;
-
- for_each_power_well(dev_priv, power_well)
- if (power_well->desc->id == power_well_id)
- return power_well;
-
- /*
-* It's not feasible to add error checking code to the callers since
-* this condition really shouldn't happen and it doesn't even make sense
-* to abort things like display initialization sequences. Just return
-* the first power well and hope the WARN gets reported so we can fix
-* our driver.
-*/
- WARN(1, "Power well %d not defined for this platform\n", power_well_id);
- return _priv->power_domains.power_wells[0];
-}
-
#define BITS_SET(val, bits) (((val) & (bits)) == (bits))
static void assert_chv_phy_status(struct drm_i915_private *dev_priv)
--
2.14.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/5] drm/i915: WARN() if we can't lookup_power_well()
None of the current lookup_power_well() callers are actually checking for NULL return values, they all just use the pointer right away. The first idea was to replace these theoretical segfaults with a BUG() since this would at least make our code a little more explicit to the reader. It was suggested that just converting the BUG() to a WARN() and returning any power well would probably be better since it would still keep the system running while at the same time exposing the driver bug. We can only hit this NULL/BUG()/WARN() condition if we try to lookup a power well that isn't defined on a given platform. If that ever happens, we have to fix our code, making it lookup the correct power well. Because of this, I don't think it's worth trying to implement error checking in every caller. Improving our CI system will be a better use of our time once a bug is found in the wild. v2: Avoid the BUG() with a WARN() return a random PW (Michal). Cc: Michal Wajdeczko Cc: Imre Deak Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/intel_runtime_pm.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index f75837e45144..f07ed70b839f 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -1096,7 +1096,15 @@ lookup_power_well(struct drm_i915_private *dev_priv, return power_well; } - return NULL; + /* +* It's not feasible to add error checking code to the callers since +* this condition really shouldn't happen and it doesn't even make sense +* to abort things like display initialization sequences. Just return +* the first power well and hope the WARN gets reported so we can fix +* our driver. +*/ + WARN(1, "Power well %d not defined for this platform\n", power_well_id); + return _domains->power_wells[0]; } #define BITS_SET(val, bits) (((val) & (bits)) == (bits)) -- 2.14.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/5] drm/i915: use for_each_power_well in lookup_power_well()
Use the nice helper function to make the implementation simpler.
v2: Rebase.
Cc: Imre Deak
Reviewed-by: José Roberto de Souza (v1)
Signed-off-by: Paulo Zanoni
---
drivers/gpu/drm/i915/intel_runtime_pm.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index f07ed70b839f..6f5a2f00a12d 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -1085,16 +1085,11 @@ static struct i915_power_well *
lookup_power_well(struct drm_i915_private *dev_priv,
enum i915_power_well_id power_well_id)
{
- struct i915_power_domains *power_domains = _priv->power_domains;
- int i;
-
- for (i = 0; i < power_domains->power_well_count; i++) {
- struct i915_power_well *power_well;
+ struct i915_power_well *power_well;
- power_well = _domains->power_wells[i];
+ for_each_power_well(dev_priv, power_well)
if (power_well->desc->id == power_well_id)
return power_well;
- }
/*
* It's not feasible to add error checking code to the callers since
@@ -1104,7 +1099,7 @@ lookup_power_well(struct drm_i915_private *dev_priv,
* our driver.
*/
WARN(1, "Power well %d not defined for this platform\n", power_well_id);
- return _domains->power_wells[0];
+ return _priv->power_domains.power_wells[0];
}
#define BITS_SET(val, bits) (((val) & (bits)) == (bits))
--
2.14.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/5] drm/i915: kill intel_display_power_well_is_enabled()
Use the same coding pattern as we use in the other functions of the
same file: just call lookup_power_well() directly in the only caller.
Cc: Imre Deak
Signed-off-by: Paulo Zanoni
---
drivers/gpu/drm/i915/intel_runtime_pm.c | 20 +++-
1 file changed, 3 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index ff3fd8dbd2b4..f75837e45144 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -49,9 +49,6 @@
* present for a given platform.
*/
-bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv,
-enum i915_power_well_id power_well_id);
-
static struct i915_power_well *
lookup_power_well(struct drm_i915_private *dev_priv,
enum i915_power_well_id power_well_id);
@@ -654,8 +651,9 @@ static void assert_csr_loaded(struct drm_i915_private
*dev_priv)
static void assert_can_enable_dc5(struct drm_i915_private *dev_priv)
{
- bool pg2_enabled = intel_display_power_well_is_enabled(dev_priv,
- SKL_DISP_PW_2);
+ struct i915_power_well *pg2 = lookup_power_well(dev_priv,
+ SKL_DISP_PW_2);
+ bool pg2_enabled = pg2->desc->ops->is_enabled(dev_priv, pg2);
WARN_ONCE(pg2_enabled, "PG2 not disabled to enable DC5.\n");
@@ -2278,18 +2276,6 @@ static const struct i915_power_well_desc
chv_power_wells[] = {
},
};
-bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv,
-enum i915_power_well_id power_well_id)
-{
- struct i915_power_well *power_well;
- bool ret;
-
- power_well = lookup_power_well(dev_priv, power_well_id);
- ret = power_well->desc->ops->is_enabled(dev_priv, power_well);
-
- return ret;
-}
-
static const struct i915_power_well_desc skl_power_wells[] = {
{
.name = "always-on",
--
2.14.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915/psr: Mask PSR irq bits when re-enabling interrupts.
On Wed, 2018-08-15 at 18:05 -0700, Dhinakaran Pandiyan wrote:
> gen8_de_irq_postinstall() wasn't masking the IRQ bit before passing
> the
> debug flag to psr_irq_control(). This check was missed when new debug
> bits
> were defined in 'commit c44301fce614 ("drm/i915: Allow control of
> PSR at
> runtime through debugfs, v6")'. Instead of ANDing the irq bit in all
> the
> callers, move it to the callee.
>
> Cc: Maarten Lankhorst
Reviewed-by: Maarten Lankhorst via IRC
"[8/20/18 11:58] or irc r-b"
> Signed-off-by: Dhinakaran Pandiyan
> ---
> drivers/gpu/drm/i915/i915_irq.c | 2 +-
> drivers/gpu/drm/i915/intel_drv.h | 2 +-
> drivers/gpu/drm/i915/intel_psr.c | 6 +++---
> 3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c
> index b2c9838442bc..8084e35b25c5 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -4048,7 +4048,7 @@ static int ironlake_irq_postinstall(struct
> drm_device *dev)
>
> if (IS_HASWELL(dev_priv)) {
> gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
> - intel_psr_irq_control(dev_priv, dev_priv->psr.debug
> & I915_PSR_DEBUG_IRQ);
> + intel_psr_irq_control(dev_priv, dev_priv-
> >psr.debug);
> display_mask |= DE_EDP_PSR_INT_HSW;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 7b984aefce98..bc1c53c5f4dd 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1944,7 +1944,7 @@ void intel_psr_flush(struct drm_i915_private
> *dev_priv,
> void intel_psr_init(struct drm_i915_private *dev_priv);
> void intel_psr_compute_config(struct intel_dp *intel_dp,
> struct intel_crtc_state *crtc_state);
> -void intel_psr_irq_control(struct drm_i915_private *dev_priv, bool
> debug);
> +void intel_psr_irq_control(struct drm_i915_private *dev_priv, u32
> debug);
> void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32
> psr_iir);
> void intel_psr_short_pulse(struct intel_dp *intel_dp);
> int intel_psr_wait_for_idle(const struct intel_crtc_state
> *new_crtc_state);
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index 7560c65f50ad..df79020045a5 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -79,7 +79,7 @@ static bool intel_psr2_enabled(struct
> drm_i915_private *dev_priv,
> }
> }
>
> -void intel_psr_irq_control(struct drm_i915_private *dev_priv, bool
> debug)
> +void intel_psr_irq_control(struct drm_i915_private *dev_priv, u32
> debug)
> {
> u32 debug_mask, mask;
>
> @@ -100,7 +100,7 @@ void intel_psr_irq_control(struct
> drm_i915_private *dev_priv, bool debug)
> EDP_PSR_PRE_ENTRY(TRANSCODER_C);
> }
>
> - if (debug)
> + if (debug & I915_PSR_DEBUG_IRQ)
> mask |= debug_mask;
>
> I915_WRITE(EDP_PSR_IMR, ~mask);
> @@ -901,7 +901,7 @@ int intel_psr_set_debugfs_mode(struct
> drm_i915_private *dev_priv,
> if (crtc)
> dev_priv->psr.psr2_enabled =
> intel_psr2_enabled(dev_priv, crtc_state);
>
> - intel_psr_irq_control(dev_priv, dev_priv->psr.debug &
> I915_PSR_DEBUG_IRQ);
> + intel_psr_irq_control(dev_priv, dev_priv->psr.debug);
>
> if (dev_priv->psr.prepared && enable)
> intel_psr_enable_locked(dev_priv, crtc_state);
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCHv3] lib/ratelimit: Lockless ratelimiting
Hi Petr, thanks for review,
On Wed, 2018-08-15 at 17:10 +0200, Petr Mladek wrote:
> On Tue 2018-07-03 23:56:28, Dmitry Safonov wrote:
> > Currently ratelimit_state is protected with spin_lock. If the .lock
> > is
> > taken at the moment of ___ratelimit() call, the message is
> > suppressed to
> > make ratelimiting robust.
> >
> > That results in the following issue issue:
> > CPU0 CPU1
> > -- --
> > printk_ratelimit() printk_ratelimit()
> > | |
> > try_spin_lock() try_spin_lock()
> > | |
> > time_is_before_jiffies() return 0; // suppress
> >
> > So, concurrent call of ___ratelimit() results in silently
> > suppressing
> > one of the messages, regardless if the limit is reached or not.
> > And rc->missed is not increased in such case so the issue is
> > covered
> > from user.
> >
> > Convert ratelimiting to use atomic counters and drop spin_lock.
> >
> > Note: That might be unexpected, but with the first interval of
> > messages
> > storm one can print up to burst*2 messages. So, it doesn't
> > guarantee
> > that in *any* interval amount of messages is lesser than burst.
> > But, that differs lightly from previous behavior where one can
> > start
> > burst=5 interval and print 4 messages on the last milliseconds of
> > interval + new 5 messages from new interval (totally 9 messages in
> > lesser than interval value):
>
> I am still confused by this paragraph. Does this patch change the
> behavior? What is the original and what is the new one, please?
Yeah, I could be a bit cleaner about the change.
Originally more than `burst` messages could be printed if the previous
period hasn't ended:
>
> >msg0 msg1-msg4 msg0-msg4
> > | | |
> > | | |
> > |--o-o-|-o|--> (t)
> > <--->
> >Lesser than burst
But now, because I check:
+ if (atomic_add_unless(>printed, 1, rs->burst))
+ return 1;
*before* checking the end of interval, the maximum number of messages
in the peak will be the same, burst*2 (burst*2 - 1 in original).
Why do I check it before the end of interval?
I thought it would be a nice optimization, making atomic_add_unless()
the only called function on the fast-path (when we haven't hit the
limit yet). If you want, I can move it into a separate patch..
> >
> > Dropped dev/random patch since v1 version:
> > lkml.kernel.org/r/<20180510125211.12583-1-d...@arista.com>
> >
> > Dropped `name' in as it's unused in RATELIMIT_STATE_INIT()
> >
> > diff --git a/lib/ratelimit.c b/lib/ratelimit.c
> > index d01f47135239..d9b749d40108 100644
> > --- a/lib/ratelimit.c
> > +++ b/lib/ratelimit.c
> > @@ -13,6 +13,18 @@
> > #include
> > #include
> >
> > +static void ratelimit_end_interval(struct ratelimit_state *rs,
> > const char *func)
> > +{
> > + rs->begin = jiffies;
> > +
> > + if (!(rs->flags & RATELIMIT_MSG_ON_RELEASE)) {
> > + unsigned int missed = atomic_xchg(>missed, 0);
> > +
> > + if (missed)
> > + pr_warn("%s: %u callbacks suppressed\n",
> > func, missed);
> > + }
> > +}
> > +
> > /*
> > * __ratelimit - rate limiting
> > * @rs: ratelimit_state data
> > @@ -27,45 +39,30 @@
> > */
> > int ___ratelimit(struct ratelimit_state *rs, const char *func)
> > {
> > - unsigned long flags;
> > - int ret;
> > -
> > if (!rs->interval)
> > return 1;
> >
> > - /*
> > -* If we contend on this state's lock then almost
> > -* by definition we are too busy to print a message,
> > -* in addition to the one that will be printed by
> > -* the entity that is holding the lock already:
> > -*/
> > - if (!raw_spin_trylock_irqsave(>lock, flags))
> > + if (unlikely(!rs->burst)) {
> > + atomic_add_unless(>missed, 1, -1);
> > + if (time_is_before_jiffies(rs->begin + rs-
> > >interval))
> > + ratelimit_end_interval(rs, func);
>
> This is racy. time_is_before_jiffies() might be valid on two
> CPUs in parallel. They would both call ratelimit_end_interval().
> This is not longer atomic context. Therefore one might get scheduled
> and set rs->begin = jiffies seconds later. I am sure that there might
> be more crazy scenarios.
That's the case with rs->burst == 0.
So, in this situation all the messages will be suppressed.
And the only reason to call ratelimit_end_interval() is to update the
start time and number of messages not printed.
I didn't want to add any complexion for this case - the worst will be
the count of messages suppressed will be imprecise for rs->burst == 0
case. Not a big deal, huh?
>
> > +
> > return 0;
> > + }
> >
> > - if (!rs->begin)
> > - rs->begin = jiffies;
> > + if
Re: [Intel-gfx] [PATCH 1/4] drm/i915: kill intel_display_power_well_is_enabled()
Em Sex, 2018-08-17 às 16:41 -0700, Paulo Zanoni escreveu:
> Em Qua, 2018-08-15 às 23:27 +0300, Imre Deak escreveu:
> > On Wed, Aug 08, 2018 at 03:16:11PM -0700, Paulo Zanoni wrote:
> > > Use the same coding pattern as we use in the other functions of
> > > the
> > > same file: just call lookup_power_well() directly in the only
> > > caller.
> > >
> > > Cc: Imre Deak
> > > Signed-off-by: Paulo Zanoni
> > > ---
> > > drivers/gpu/drm/i915/intel_runtime_pm.c | 20 +++--
> > > ---
> > > 1 file changed, 3 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > index e209edbc561d..e0947f662361 100644
> > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > @@ -49,9 +49,6 @@
> > > * present for a given platform.
> > > */
> > >
> > > -bool intel_display_power_well_is_enabled(struct drm_i915_private
> > > *dev_priv,
> > > - enum i915_power_well_id
> > > power_well_id);
> > > -
> > > static struct i915_power_well *
> > > lookup_power_well(struct drm_i915_private *dev_priv,
> > > enum i915_power_well_id power_well_id);
> > > @@ -678,8 +675,9 @@ static void assert_csr_loaded(struct
> > > drm_i915_private *dev_priv)
> > >
> > > static void assert_can_enable_dc5(struct drm_i915_private
> > > *dev_priv)
> > > {
> > > - bool pg2_enabled =
> > > intel_display_power_well_is_enabled(dev_priv,
> > > - SKL_DISP_PW_2);
> > > + struct i915_power_well *pg2 =
> > > lookup_power_well(dev_priv,
> > > + SKL_DISP
> > > _P
> > > W_2);
> > > + bool pg2_enabled = pg2->desc->ops->is_enabled(dev_priv,
> > > pg2);
> > >
> > > WARN_ONCE(pg2_enabled, "PG2 not disabled to enable
> > > DC5.\n");
> > >
> > > @@ -2302,18 +2300,6 @@ static const struct i915_power_well_desc
> > > chv_power_wells[] = {
> > > },
> > > };
> > >
> > > -bool intel_display_power_well_is_enabled(struct drm_i915_private
> > > *dev_priv,
> > > - enum i915_power_well_id
> > > power_well_id)
> > > -{
> > > - struct i915_power_well *power_well;
> > > - bool ret;
> > > -
> > > - power_well = lookup_power_well(dev_priv, power_well_id);
> > > - ret = power_well->desc->ops->is_enabled(dev_priv,
> > > power_well);
> > > -
> > > - return ret;
> > > -}
> > > -
> >
> > Or rather export a locked version of it and use that in
> > intel_hdcp.c
> > to
> > better hide the internals?
>
> That should probably be combined with José's idea of using ->enabled
> so
> we trust the hardware sync.
>
> Thanks for the suggestions.
After further analysis, I wonder if intel_hdcp.c should really be
checking for enabled power wells or if it should be doing something
else, such as actually grabbing power domain references to make sure
we're able to enable/disable HDCP whenever we need. Most of our code
should not be checking for power wells/domains being enabled/disabled
(except for HW readout), it should actually be requesting those
resources to make sure we have them when we need them.
CCing Ramaligam for that.
>
> >
> > > static const struct i915_power_well_desc skl_power_wells[] = {
> > > {
> > > .name = "always-on",
> > > --
> > > 2.14.4
> > >
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/5] drm/dp/fec: DRM helper for Forward Error Correction
Hi Jani,
Thanks for your feedback. Please see my comments below:
On Fri, Aug 17, 2018 at 12:51:03PM +0300, Jani Nikula wrote:
> On Tue, 07 Aug 2018, Anusha Srivatsa wrote:
> > From: "Srivatsa, Anusha"
> >
> > DP 1.4 has Forward Error Correction Support(FEC).
> > Add helper function to check if the sink device
> > supports FEC.
> >
> > v2: Separate the helper and the code that uses the helper into
> > two separate patches. (Manasi)
> >
> > v3:
> > - Move the code to drm_dp_helper.c (Manasi)
> > - change the return type, code style changes (Gaurav)
> > - Use drm_dp_dpcd_readb instead of drm_dp_dpcd_read. (Jani)
> >
> > Cc: Ville Syrjala
> > Cc: Jani Nikula
> > Cc: Manasi Navare
> > Signed-off-by: Anusha Srivatsa
> > ---
> > drivers/gpu/drm/drm_dp_helper.c | 14 ++
> > include/drm/drm_dp_helper.h | 3 +++
>
> Neither of these is i915, thus intel-gfx is not enough.
>
> > 2 files changed, 17 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_dp_helper.c
> > b/drivers/gpu/drm/drm_dp_helper.c
> > index 7dc61d1..2e127b9 100644
> > --- a/drivers/gpu/drm/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > @@ -1425,3 +1425,17 @@ u8 drm_dp_dsc_sink_max_color_depth(const u8
> > dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> > return 0;
> > }
> > EXPORT_SYMBOL(drm_dp_dsc_sink_max_color_depth);
> > +
> > +/* Forward Error Correction support for DP 1.4 */
> > +int drm_dp_sink_supports_fec(struct drm_dp_aux *aux)
> > +{
> > + int fec_err;
> > + u8 fec_cap;
> > +
> > + fec_err = drm_dp_dpcd_readb(aux, DP_FEC_CAPABILITY, _cap);
> > + if (fec_err < 0)
> > + return fec_err;
> > +
> > + return fec_cap & DP_FEC_CAPABLE;
>
> I can't help but think this function feels wrong in so many ways.
>
> First, how does this function fit with the rest of the helpers? Most
> helpers operate on previously read data. At the very least the function
> name should reflect the fact that this *reads* DPCD *if* this continues
> to read the DPCD.
I agree that this does not fit with rest of the helpers. All the other
helpers that get any information from DPCD registers actually work
on the cached set of registers.
So here to follow the same format, we could cache FEC DPCD registers -
FEC_SUPPORT, FEC_CONFIGURATION, FEC_STATUS, FEC_ERROR_COUNT.
Out fo which we really for now need to read FEC_SUPPORT but might
need other registers in the future.
One way to correct this would be to cache these at the same time when
we cache the DSC DPCD registers into say fec_dpcd[].
That way we dont trigger the aux reads everytime we need to get fec_support()
Jani, does this sound like a good solution?
>
> Second, what are you going to do when you need to read the other bits in
> the same register? Read it again in the driver? Add more helpers? But
> you only want to read the register *once*. This one seems too
> specialized.
>
> Third, the name implies a boolean return, but it's not. An unsuspecting
> caller will use this and get a "supports fec" return on errors. This is
> hard to use. Patch 2/5 in this series makes that exact mistake twice,
> it's the first user, and sets the example.
Yes I think this should follow the same format as drm_dp_sink_supports_dsc()
where
it returns a bool based on the cached value.
Manasi
>
> I'm not convinced of the usefulness of this particular helper.
>
> BR,
> Jani.
>
>
>
> > +}
> > +EXPORT_SYMBOL(drm_dp_sink_supports_fec);
> > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> > index f178933..d958c7d 100644
> > --- a/include/drm/drm_dp_helper.h
> > +++ b/include/drm/drm_dp_helper.h
> > @@ -1270,6 +1270,9 @@ void drm_dp_aux_unregister(struct drm_dp_aux *aux);
> > int drm_dp_start_crc(struct drm_dp_aux *aux, struct drm_crtc *crtc);
> > int drm_dp_stop_crc(struct drm_dp_aux *aux);
> >
> > +/* Forward Error Correction Support on DP 1.4 */
> > +int drm_dp_sink_supports_fec(struct drm_dp_aux *aux);
> > +
> > struct drm_dp_dpcd_ident {
> > u8 oui[3];
> > u8 device_id[6];
>
> --
> Jani Nikula, Intel Open Source Graphics Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Show debugfs dpcd read failure directly
On Mon, 2018-08-20 at 10:15 +0300, Jani Nikula wrote:
> On Fri, 20 Jul 2018, Rodrigo Vivi wrote:
> > Instead of using a backchannel if some dpcd read failed we
> > can show that directly on debugfs output.
> >
> > We are not returning an error because we might still want
> > to know if sub-sequent reads works, but we shouldn't
> > need to check 2 places to see why reg is not on the list.
>
> Should we just nuke this debugfs and use the aux chardev interface to
> dpcd instead?
Given that this debugfs does not print decoded information, I think, it
offers very little benefit over direct reads. We also print some DPCD's
in dmesg, so I'm in favour of killing it. Don't see the file being used
in IGTs either.
Your comment also reminds about the IGT tool that Tarun started writing
for reading DPCD.
-DK
>
> BR,
> Jani.
>
> >
> > Cc: Jani Nikula
> > Cc: Dhinakaran Pandiyan
> > Signed-off-by: Rodrigo Vivi
> > ---
> > drivers/gpu/drm/i915/i915_debugfs.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 59dc0610ea44..5d8da4e8c444 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -4846,8 +4846,8 @@ static int i915_dpcd_show(struct seq_file *m,
> > void *data)
> >
> > err = drm_dp_dpcd_read(_dp->aux, b->offset,
> > buf, size);
> > if (err <= 0) {
> > - DRM_ERROR("dpcd read (%zu bytes at %u)
> > failed (%zd)\n",
> > - size, b->offset, err);
> > + seq_printf(m, "dpcd read (%zu bytes at %u)
> > failed (%zd)\n",
> > + size, b->offset, err);
> > continue;
> > }
>
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/audio: Hook up component bindings even if displays are disabled
Quoting Imre Deak (2018-08-20 14:23:13)
> On Fri, Aug 17, 2018 at 11:02:41AM +0100, Chris Wilson wrote:
> > If the display has been disabled by modparam, we still want to connect
> > together the HW bits and bobs with the associated drivers so that we can
> > continue to manage their runtime power gating.
> >
> > Fixes: 108109444ff6 ("drm/i915: Check num_pipes before initializing audio
> > component")
> > Signed-off-by: Chris Wilson
> > Cc: Imre Deak
> > Cc: Takashi Iwai
> > Cc: Jani Nikula
> > Cc: Elaine Wang
>
> Going through the hooks in i915_audio_component_ops, I haven't noticed
> anything that would actually go wrong with num_pipes=0. Besides some
> audio HW register programming from i915_audio_component_codec_wake_override(),
> we'd skip doing everything else due to not reporting any active encoders
> to the audio component (as we do during normal display modeset). So
>
> Reviewed-by: Imre Deak
I'm confident in that we at least exercise the no-pipes case, less
confident that we test for fused-off hw. However, the audio component
simply should not find any output channels even in that case, so it
indeed should be safe.
Cross your fingers, thanks for the review,
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 i-g-t 0/2] Fix some GCC warnings
Hi, I would like to know if there is any improvement that I can do for this patchset. Thanks On 07/29, Rodrigo Siqueira wrote: > During the compilation, some GCC (8.1) warnings are shown. This patchset > address some of the warnings problems. > > Changes since v2: > - Reduce the total amount of bytes allocated during command handling >operation > - Remove unnecessary memset operation > - Improved commit message > > Rodrigo Siqueira (2): > Make string commands dynamic allocate > Fix truncate string in the snprintf > > tools/intel_gvtg_test.c | 25 - > tools/intel_reg.c | 2 +- > 2 files changed, 17 insertions(+), 10 deletions(-) > > -- > 2.17.0 > -- Rodrigo Siqueira http://siqueira.tech Graduate Student Department of Computer Science University of São Paulo ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 0/2] Add support to force specific module load
Hi, I would like to know if there is any improvement that I can do for this patchset. Thanks On 07/07, Rodrigo Siqueira wrote: > This patchset adds support for forcing a specific module to be loaded > via command line. This feature can bring benefits for developers that > need to create a new module since they can use IGT as a base test to > guide their development (Test-driven development - TDD ). The first > patch, expand the size of the module name accepted by IGT to make > possible that modules with larger names can be loaded. The second patch > has the force option implementation. > > The discussion about this feature started with an RFC named "[PATCH > i-g-t] [RFC] Add support to force specific module load", link: > https://www.spinics.net/lists/intel-gfx/msg166764.html > > Rodrigo Siqueira (2): > Increase the string size for a module name > Add support for forcing specific module > > lib/drmtest.c| 39 +++ > lib/igt_core.c | 23 +++ > lib/igt_core.h | 4 > scripts/run-tests.sh | 4 +++- > 4 files changed, 61 insertions(+), 9 deletions(-) > > -- > 2.18.0 > -- Rodrigo Siqueira http://siqueira.tech Graduate Student Department of Computer Science University of São Paulo ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/audio: Hook up component bindings even if displays are disabled
On Fri, Aug 17, 2018 at 11:02:41AM +0100, Chris Wilson wrote:
> If the display has been disabled by modparam, we still want to connect
> together the HW bits and bobs with the associated drivers so that we can
> continue to manage their runtime power gating.
>
> Fixes: 108109444ff6 ("drm/i915: Check num_pipes before initializing audio
> component")
> Signed-off-by: Chris Wilson
> Cc: Imre Deak
> Cc: Takashi Iwai
> Cc: Jani Nikula
> Cc: Elaine Wang
Going through the hooks in i915_audio_component_ops, I haven't noticed
anything that would actually go wrong with num_pipes=0. Besides some
audio HW register programming from i915_audio_component_codec_wake_override(),
we'd skip doing everything else due to not reporting any active encoders
to the audio component (as we do during normal display modeset). So
Reviewed-by: Imre Deak
> ---
> drivers/gpu/drm/i915/intel_audio.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> index b725835b47ef..769f3f586661 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -962,9 +962,6 @@ void i915_audio_component_init(struct drm_i915_private
> *dev_priv)
> {
> int ret;
>
> - if (INTEL_INFO(dev_priv)->num_pipes == 0)
> - return;
> -
> ret = component_add(dev_priv->drm.dev, _audio_component_bind_ops);
> if (ret < 0) {
> DRM_ERROR("failed to add audio component (%d)\n", ret);
> --
> 2.18.0
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Increase LSPCON timeout
On Mon, 20 Aug 2018, "Sharma, Shashank" wrote:
> On 8/18/2018 1:37 AM, Fredrik Schön wrote:
>
>> 100 ms is not enough time for the LSPCON adapter on Intel NUC devices to
>> settle. This causes dropped display modes at boot or screen reconfiguration.
>> Empirical testing can reproduce the error up to a timeout of 190 ms. Basic
>> boot and stress testing at 200 ms has not (yet) failed.
>>
>> Increase timeout to 400 ms to get some margin of error.
>>
>> Changes from v1:
>> The initial suggestion of 1000 ms was lowered due to concerns about delaying
>> valid timeout cases.
>> Update patch metadata.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107503
>> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1570392
>> Fixes: 357c0ae9198a ("drm/i915/lspcon: Wait for expected LSPCON mode to
>> settle")
>> Cc: Shashank Sharma
>> Cc: Imre Deak
>> Cc: Jani Nikula
>> Cc: # v4.11+
>> Reviewed-by: Rodrigo Vivi
>> Signed-off-by: Fredrik Schön
>> ---
>> drivers/gpu/drm/i915/intel_lspcon.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c
>> b/drivers/gpu/drm/i915/intel_lspcon.c
>> index 8ae8f42f430a..6b6758419fb3 100644
>> --- a/drivers/gpu/drm/i915/intel_lspcon.c
>> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
>> @@ -74,7 +74,7 @@ static enum drm_lspcon_mode lspcon_wait_mode(struct
>> intel_lspcon *lspcon,
>> DRM_DEBUG_KMS("Waiting for LSPCON mode %s to settle\n",
>>lspcon_mode_name(mode));
>>
>> -wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode, 100);
>> +wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode, 400);
>> if (current_mode != mode)
>> DRM_ERROR("LSPCON mode hasn't settled\n");
>>
> Looks good to me
> Reviewed-by: Shashank Sharma
Pushed to drm-intel-next-queued, thanks for the patch and review.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Increase LSPCON timeout
On 8/18/2018 1:37 AM, Fredrik Schön wrote:
100 ms is not enough time for the LSPCON adapter on Intel NUC devices to
settle. This causes dropped display modes at boot or screen reconfiguration.
Empirical testing can reproduce the error up to a timeout of 190 ms. Basic
boot and stress testing at 200 ms has not (yet) failed.
Increase timeout to 400 ms to get some margin of error.
Changes from v1:
The initial suggestion of 1000 ms was lowered due to concerns about delaying
valid timeout cases.
Update patch metadata.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107503
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1570392
Fixes: 357c0ae9198a ("drm/i915/lspcon: Wait for expected LSPCON mode to settle")
Cc: Shashank Sharma
Cc: Imre Deak
Cc: Jani Nikula
Cc: # v4.11+
Reviewed-by: Rodrigo Vivi
Signed-off-by: Fredrik Schön
---
drivers/gpu/drm/i915/intel_lspcon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_lspcon.c
b/drivers/gpu/drm/i915/intel_lspcon.c
index 8ae8f42f430a..6b6758419fb3 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -74,7 +74,7 @@ static enum drm_lspcon_mode lspcon_wait_mode(struct
intel_lspcon *lspcon,
DRM_DEBUG_KMS("Waiting for LSPCON mode %s to settle\n",
lspcon_mode_name(mode));
- wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode, 100);
+ wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode, 400);
if (current_mode != mode)
DRM_ERROR("LSPCON mode hasn't settled\n");
Looks good to me
Reviewed-by: Shashank Sharma
- Shashank
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Verify power domains after enabling them (rev3)
On Fri, Aug 17, 2018 at 03:24:40PM +, Patchwork wrote:
> == Series Details ==
>
> Series: drm/i915: Verify power domains after enabling them (rev3)
> URL : https://patchwork.freedesktop.org/series/48394/
> State : success
Pushed to -dinq with i915_welcome_messages() updated, thanks for the
review.
>
> == Summary ==
>
> = CI Bug Log - changes from CI_DRM_4685 -> Patchwork_9973 =
>
> == Summary - SUCCESS ==
>
> No regressions found.
>
> External URL:
> https://patchwork.freedesktop.org/api/1.0/series/48394/revisions/3/mbox/
>
> == Possible new issues ==
>
> Here are the unknown changes that may have been introduced in
> Patchwork_9973:
>
> === IGT changes ===
>
> Warnings
>
> {igt@pm_rpm@module-reload}:
> {fi-icl-u}: WARN (fdo#107602) -> DMESG-FAIL
>
>
> == Known issues ==
>
> Here are the changes found in Patchwork_9973 that come from known issues:
>
> === IGT changes ===
>
> Issues hit
>
> igt@drv_selftest@live_guc:
> {fi-icl-u}: PASS -> DMESG-WARN (fdo#107591) +14
>
> igt@drv_selftest@live_hangcheck:
> fi-skl-6600u: PASS -> DMESG-FAIL (fdo#106560, fdo#107174)
>
> igt@gem_exec_suspend@basic-s4-devices:
> fi-kbl-7500u: PASS -> DMESG-WARN (fdo#107139, fdo#105128)
>
> igt@kms_frontbuffer_tracking@basic:
> {fi-byt-clapper}: PASS -> FAIL (fdo#103167)
>
> igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
> {fi-byt-clapper}: PASS -> FAIL (fdo#107362)
>
> {igt@pm_rpm@module-reload}:
> fi-byt-j1900: NOTRUN -> WARN (fdo#107602)
>
>
> Possible fixes
>
> igt@drv_selftest@live_hangcheck:
> fi-skl-guc: DMESG-FAIL (fdo#107174) -> PASS
>
> igt@gem_exec_basic@readonly-blt:
> fi-byt-n2820: FAIL (fdo#105900) -> PASS
>
> igt@kms_frontbuffer_tracking@basic:
> fi-hsw-peppy: DMESG-FAIL (fdo#102614) -> PASS
>
> igt@prime_vgem@basic-fence-flip:
> fi-ivb-3770:FAIL (fdo#104008) -> PASS
>
>
> {name}: This element is suppressed. This means it is ignored when computing
> the status of the difference (SUCCESS, WARNING, or FAILURE).
>
> fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
> fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
> fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
> fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
> fdo#105900 https://bugs.freedesktop.org/show_bug.cgi?id=105900
> fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
> fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139
> fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174
> fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
> fdo#107591 https://bugs.freedesktop.org/show_bug.cgi?id=107591
> fdo#107602 https://bugs.freedesktop.org/show_bug.cgi?id=107602
>
>
> == Participating hosts (52 -> 49) ==
>
> Additional (1): fi-byt-j1900
> Missing(4): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-hsw-4200u
>
>
> == Build changes ==
>
> * Linux: CI_DRM_4685 -> Patchwork_9973
>
> CI_DRM_4685: df7e8eddc3830216d3fec15e2c7d0b6ec97e7bae @
> git://anongit.freedesktop.org/gfx-ci/linux
> IGT_4606: 38a44003774e35c587c67c8766b35e75dbb993b8 @
> git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
> Patchwork_9973: c7bd0b63b5794846f480ad271ce92866cb41c5eb @
> git://anongit.freedesktop.org/gfx-ci/linux
>
>
> == Linux commits ==
>
> c7bd0b63b579 drm/i915: Verify power domains after enabling them
>
> == Logs ==
>
> For more details see:
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9973/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Show debugfs dpcd read failure directly
On Fri, 20 Jul 2018, Rodrigo Vivi wrote:
> Instead of using a backchannel if some dpcd read failed we
> can show that directly on debugfs output.
>
> We are not returning an error because we might still want
> to know if sub-sequent reads works, but we shouldn't
> need to check 2 places to see why reg is not on the list.
Should we just nuke this debugfs and use the aux chardev interface to
dpcd instead?
BR,
Jani.
>
> Cc: Jani Nikula
> Cc: Dhinakaran Pandiyan
> Signed-off-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 59dc0610ea44..5d8da4e8c444 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4846,8 +4846,8 @@ static int i915_dpcd_show(struct seq_file *m, void
> *data)
>
> err = drm_dp_dpcd_read(_dp->aux, b->offset, buf, size);
> if (err <= 0) {
> - DRM_ERROR("dpcd read (%zu bytes at %u) failed (%zd)\n",
> - size, b->offset, err);
> + seq_printf(m, "dpcd read (%zu bytes at %u) failed
> (%zd)\n",
> +size, b->offset, err);
> continue;
> }
--
Jani Nikula, Intel Open Source Graphics Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/dp: Link train Fallback on eDP only if fallback link BW can fit panel's native mode
I believe this is the patch discussed here:
https://bugs.freedesktop.org/show_bug.cgi?id=105338
In which case
Tested-By: Alexander Wilson
On 8/17/18 4:36 PM, Manasi Navare wrote:
> Thanks Lyude.
>
> So based on the imitial comments from Jani N, the recommendation was to
> disconnect
> downclock_mode from drrs_init so that user can set downclock mode
> independently from drrs mode.
>
> Jani,
> So we would need following changes:
> * Set the panel->downclock_mode in edp_init_connector() using
> intel_find_panel_downclock()
> * Add downclock_mode->clock when we check against available BW in mode_valid
> * Also use that in compute_config
> * Then check against downclock_mode on link training fallback
>
> Any more changes recommended here?
>
> Manasi
>
> On Fri, Aug 17, 2018 at 04:43:22PM -0400, Lyude Paul wrote:
>> On Fri, 2018-08-17 at 13:40 -0700, Manasi Navare wrote:
>>> On Fri, Aug 17, 2018 at 04:32:09PM -0400, Lyude Paul wrote:
After reading the discussion so far on this patch, this sounds correct! One
nit
pick below though:
On Wed, 2018-05-16 at 19:21 -0700, Manasi Navare wrote:
> This patch fixes the original commit c0cfb10d9e1de49 ("drm/i915/edp:
> Do not do link training fallback or prune modes on EDP") that causes
> a blank screen in case of certain eDP panels (Eg: seen on Dell XPS13 9350)
> where first link training fails and a retraining is required by falling
> back to lower link rate/lane count.
> In case of some panels they advertise higher link rate/lane count
> than whats required for supporting the panel's native mode.
> But we always link train at highest link rate/lane count for eDP
> and if that fails we can still fallback to lower link rate/lane count
> as long as the fallback link BW still fits the native mode to avoid
> pruning the panel's native mode yet retraining at fallback values
> to recover from a blank screen.
>
> Cc: Clinton Taylor
> Cc: Jani Nikula
> Cc: Ville Syrjala
> Cc: Daniel Vetter
> Cc: Lucas De Marchi
> Signed-off-by: Manasi Navare
> ---
> drivers/gpu/drm/i915/intel_dp.c | 25
> +
> drivers/gpu/drm/i915/intel_dp_link_training.c | 26 +-
>
> 2 files changed, 34 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index 2cc58596..7f7202a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -387,6 +387,21 @@ static bool intel_dp_link_params_valid(struct
> intel_dp
> *intel_dp, int link_rate,
> return true;
> }
>
> +static bool intel_dp_can_link_train_fallback_for_edp(struct intel_dp
> *intel_dp,
> + int link_rate,
> + uint8_t lane_count)
> +{
> + struct drm_display_mode *fixed_mode = intel_dp->attached_connector-
>> panel.fixed_mode;
> + int mode_rate, max_rate;
> +
> + mode_rate = intel_dp_link_required(fixed_mode->clock, 18);
> + max_rate = intel_dp_max_data_rate(link_rate, lane_count);
> + if (mode_rate > max_rate)
> + return false;
> +
> + return true;
> +}
> +
> int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> int link_rate, uint8_t lane_count)
> {
> @@ -396,9 +411,19 @@ int intel_dp_get_link_train_fallback_values(struct
> intel_dp *intel_dp,
> intel_dp->num_common_rates,
> link_rate);
> if (index > 0) {
> + if (intel_dp_is_edp(intel_dp) &&
> + !intel_dp_can_link_train_fallback_for_edp(intel_dp,
> + intel_dp-
>> common_rates[index-1],
> + lane_count))
> + return -1;
> intel_dp->max_link_rate = intel_dp->common_rates[index - 1];
> intel_dp->max_link_lane_count = lane_count;
> } else if (lane_count > 1) {
> + if (intel_dp_is_edp(intel_dp) &&
> + !intel_dp_can_link_train_fallback_for_edp(intel_dp,
> + intel_dp_max_commo
> n_rate(intel_dp),
> + lane_count >> 1))
> + return -1;
The arguments you pass to intel_dp_can_link_train_fallback_for_edp() are
the
same ones that you assign to intel_dp->max_link_rate and intel_dp-
> max_link_lane_count, why not just set those latter two first then pass
> them to
intel_dp_can_link_train_fallback_for_edp() afterwards?
>>> Actually I had thought of that. However if the
>>>
