* [PATCH] intel/atomic: Stop updating legacy fb parameters
@ 2017-12-05 17:00 Daniel Vetter
2017-12-05 17:40 ` Ville Syrjälä
2017-12-05 18:10 ` ✗ Fi.CI.BAT: failure for " Patchwork
0 siblings, 2 replies; 7+ messages in thread
From: Daniel Vetter @ 2017-12-05 17:00 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter, Paulo Zanoni, Daniel Vetter
Even fbc isn't using this stuff anymore, so time to remove it.
Cleaning up one small piece of the atomic conversion cruft at the time
...
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 29 -----------------------------
1 file changed, 29 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1f7e312d0d0d..c883e14a06d3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10967,31 +10967,6 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
return ret;
}
-static void
-intel_modeset_update_crtc_state(struct drm_atomic_state *state)
-{
- struct drm_crtc *crtc;
- struct drm_crtc_state *new_crtc_state;
- int i;
-
- /* Double check state. */
- for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
- to_intel_crtc(crtc)->config = to_intel_crtc_state(new_crtc_state);
-
- /*
- * Update legacy state to satisfy fbc code. This can
- * be removed when fbc uses the atomic state.
- */
- if (drm_atomic_get_existing_plane_state(state, crtc->primary)) {
- struct drm_plane_state *plane_state = crtc->primary->state;
-
- crtc->primary->fb = plane_state->fb;
- crtc->x = plane_state->src_x >> 16;
- crtc->y = plane_state->src_y >> 16;
- }
- }
-}
-
static bool intel_fuzzy_clock_check(int clock1, int clock2)
{
int diff;
@@ -12364,10 +12339,6 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
}
}
- /* Only after disabling all output pipelines that will be changed can we
- * update the the output configuration. */
- intel_modeset_update_crtc_state(state);
-
if (intel_state->modeset) {
drm_atomic_helper_update_legacy_modeset_state(state->dev, state);
--
2.15.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] intel/atomic: Stop updating legacy fb parameters
2017-12-05 17:00 [PATCH] intel/atomic: Stop updating legacy fb parameters Daniel Vetter
@ 2017-12-05 17:40 ` Ville Syrjälä
2017-12-06 10:31 ` Maarten Lankhorst
2017-12-05 18:10 ` ✗ Fi.CI.BAT: failure for " Patchwork
1 sibling, 1 reply; 7+ messages in thread
From: Ville Syrjälä @ 2017-12-05 17:40 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development, Paulo Zanoni
On Tue, Dec 05, 2017 at 06:00:20PM +0100, Daniel Vetter wrote:
> Even fbc isn't using this stuff anymore, so time to remove it.
>
> Cleaning up one small piece of the atomic conversion cruft at the time
> ...
>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 29 -----------------------------
> 1 file changed, 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1f7e312d0d0d..c883e14a06d3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10967,31 +10967,6 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
> return ret;
> }
>
> -static void
> -intel_modeset_update_crtc_state(struct drm_atomic_state *state)
> -{
> - struct drm_crtc *crtc;
> - struct drm_crtc_state *new_crtc_state;
> - int i;
> -
> - /* Double check state. */
> - for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
> - to_intel_crtc(crtc)->config = to_intel_crtc_state(new_crtc_state);
> -
> - /*
> - * Update legacy state to satisfy fbc code. This can
> - * be removed when fbc uses the atomic state.
> - */
> - if (drm_atomic_get_existing_plane_state(state, crtc->primary)) {
> - struct drm_plane_state *plane_state = crtc->primary->state;
> -
> - crtc->primary->fb = plane_state->fb;
> - crtc->x = plane_state->src_x >> 16;
> - crtc->y = plane_state->src_y >> 16;
crtc->x/y seem pretty safe to nuke here.
Not quite sure about plane->fb. The core still messes around with that
quite a bit. The lack of refcounting here seems strange too. I've been
actually wondering if that mess is somehow related to the fb leak we
have in CI on some machines.
> - }
> - }
> -}
> -
> static bool intel_fuzzy_clock_check(int clock1, int clock2)
> {
> int diff;
> @@ -12364,10 +12339,6 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
> }
> }
>
> - /* Only after disabling all output pipelines that will be changed can we
> - * update the the output configuration. */
> - intel_modeset_update_crtc_state(state);
> -
> if (intel_state->modeset) {
> drm_atomic_helper_update_legacy_modeset_state(state->dev, state);
>
> --
> 2.15.0
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] intel/atomic: Stop updating legacy fb parameters
2017-12-05 17:40 ` Ville Syrjälä
@ 2017-12-06 10:31 ` Maarten Lankhorst
0 siblings, 0 replies; 7+ messages in thread
From: Maarten Lankhorst @ 2017-12-06 10:31 UTC (permalink / raw)
To: Ville Syrjälä, Daniel Vetter
Cc: Daniel Vetter, Intel Graphics Development, Paulo Zanoni
Op 05-12-17 om 18:40 schreef Ville Syrjälä:
> On Tue, Dec 05, 2017 at 06:00:20PM +0100, Daniel Vetter wrote:
>> Even fbc isn't using this stuff anymore, so time to remove it.
>>
>> Cleaning up one small piece of the atomic conversion cruft at the time
>> ...
>>
>> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_display.c | 29 -----------------------------
>> 1 file changed, 29 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 1f7e312d0d0d..c883e14a06d3 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -10967,31 +10967,6 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
>> return ret;
>> }
>>
>> -static void
>> -intel_modeset_update_crtc_state(struct drm_atomic_state *state)
>> -{
>> - struct drm_crtc *crtc;
>> - struct drm_crtc_state *new_crtc_state;
>> - int i;
>> -
>> - /* Double check state. */
>> - for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
>> - to_intel_crtc(crtc)->config = to_intel_crtc_state(new_crtc_state);
>> -
>> - /*
>> - * Update legacy state to satisfy fbc code. This can
>> - * be removed when fbc uses the atomic state.
>> - */
>> - if (drm_atomic_get_existing_plane_state(state, crtc->primary)) {
>> - struct drm_plane_state *plane_state = crtc->primary->state;
>> -
>> - crtc->primary->fb = plane_state->fb;
>> - crtc->x = plane_state->src_x >> 16;
>> - crtc->y = plane_state->src_y >> 16;
> crtc->x/y seem pretty safe to nuke here.
>
> Not quite sure about plane->fb. The core still messes around with that
> quite a bit. The lack of refcounting here seems strange too. I've been
> actually wondering if that mess is somehow related to the fb leak we
> have in CI on some machines.
>
>> - }
>> - }
>> -}
>> -
>> static bool intel_fuzzy_clock_check(int clock1, int clock2)
>> {
>> int diff;
>> @@ -12364,10 +12339,6 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
>> }
>> }
>>
>> - /* Only after disabling all output pipelines that will be changed can we
>> - * update the the output configuration. */
>> - intel_modeset_update_crtc_state(state);
>> -
>> if (intel_state->modeset) {
>> drm_atomic_helper_update_legacy_modeset_state(state->dev, state);
>>
>> --
>> 2.15.0
I don't think we should stop updating crtc->config, please move that loop to where intel_modeset_update_crtc_state() and you can have my r-b.
I already had a patch in my tree to do the same, but lost it.
plane->fb is safe to nuke, the atomic core updates it but we needed it earlier for FBC and back-then-not-quite atomic plane_update calls..
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* ✗ Fi.CI.BAT: failure for intel/atomic: Stop updating legacy fb parameters
2017-12-05 17:00 [PATCH] intel/atomic: Stop updating legacy fb parameters Daniel Vetter
2017-12-05 17:40 ` Ville Syrjälä
@ 2017-12-05 18:10 ` Patchwork
1 sibling, 0 replies; 7+ messages in thread
From: Patchwork @ 2017-12-05 18:10 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
== Series Details ==
Series: intel/atomic: Stop updating legacy fb parameters
URL : https://patchwork.freedesktop.org/series/34924/
State : failure
== Summary ==
Series 34924v1 intel/atomic: Stop updating legacy fb parameters
https://patchwork.freedesktop.org/api/1.0/series/34924/revisions/1/mbox/
Test core_auth:
Subgroup basic-auth:
pass -> INCOMPLETE (fi-gdg-551)
pass -> INCOMPLETE (fi-ivb-3770)
pass -> DMESG-WARN (fi-bsw-n3050)
pass -> INCOMPLETE (fi-skl-6700hq)
Test core_prop_blob:
Subgroup basic:
pass -> DMESG-WARN (fi-bsw-n3050)
Test debugfs_test:
Subgroup read_all_entries:
dmesg-warn -> INCOMPLETE (fi-elk-e7500) fdo#103989
pass -> INCOMPLETE (fi-snb-2600)
pass -> FAIL (fi-ivb-3520m)
pass -> INCOMPLETE (fi-bdw-5557u)
pass -> INCOMPLETE (fi-bsw-n3050)
pass -> INCOMPLETE (fi-skl-6260u)
pass -> INCOMPLETE (fi-skl-6600u)
pass -> INCOMPLETE (fi-bxt-j4205)
pass -> INCOMPLETE (fi-kbl-7560u)
pass -> INCOMPLETE (fi-kbl-7567u)
pass -> INCOMPLETE (fi-kbl-r)
pass -> INCOMPLETE (fi-glk-1)
Test drv_hangman:
Subgroup error-state-basic:
pass -> INCOMPLETE (fi-blb-e6850)
pass -> DMESG-WARN (fi-pnv-d510)
pass -> INCOMPLETE (fi-bwr-2160)
Test gem_busy:
Subgroup basic-hang-default:
pass -> DMESG-WARN (fi-pnv-d510)
Test gem_exec_fence:
Subgroup await-hang-default:
pass -> DMESG-WARN (fi-pnv-d510)
Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
pass -> DMESG-WARN (fi-pnv-d510)
Test gem_exec_suspend:
Subgroup basic-s3:
pass -> DMESG-WARN (fi-pnv-d510)
pass -> INCOMPLETE (fi-ivb-3520m)
pass -> INCOMPLETE (fi-hsw-4770)
pass -> DMESG-WARN (fi-hsw-4770r)
pass -> INCOMPLETE (fi-bxt-dsi)
Subgroup basic-s4-devices:
pass -> DMESG-WARN (fi-pnv-d510)
Test gem_sync:
Subgroup basic-all:
pass -> DMESG-WARN (fi-pnv-d510)
Test kms_busy:
Subgroup basic-flip-a:
pass -> DMESG-WARN (fi-pnv-d510)
Subgroup basic-flip-b:
pass -> DMESG-FAIL (fi-pnv-d510)
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-legacy:
pass -> DMESG-WARN (fi-pnv-d510)
Subgroup basic-flip-after-cursor-legacy:
pass -> DMESG-WARN (fi-pnv-d510)
Subgroup basic-flip-after-cursor-varying-size:
pass -> DMESG-WARN (fi-pnv-d510)
Subgroup basic-flip-before-cursor-legacy:
pass -> DMESG-WARN (fi-pnv-d510)
Subgroup basic-flip-before-cursor-varying-size:
pass -> DMESG-WARN (fi-pnv-d510)
Test kms_flip:
Subgroup basic-flip-vs-dpms:
pass -> INCOMPLETE (fi-pnv-d510)
fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fi-bdw-5557u total:3 pass:2 dwarn:0 dfail:0 fail:0 skip:0
fi-blb-e6850 total:6 pass:5 dwarn:0 dfail:0 fail:0 skip:0
fi-bsw-n3050 total:3 pass:0 dwarn:2 dfail:0 fail:0 skip:0
fi-bwr-2160 total:6 pass:5 dwarn:0 dfail:0 fail:0 skip:0
fi-bxt-dsi total:108 pass:95 dwarn:0 dfail:0 fail:0 skip:12
fi-bxt-j4205 total:3 pass:2 dwarn:0 dfail:0 fail:0 skip:0
fi-elk-e7500 total:3 pass:2 dwarn:0 dfail:0 fail:0 skip:0
fi-gdg-551 total:1 pass:0 dwarn:0 dfail:0 fail:0 skip:0
fi-glk-1 total:3 pass:2 dwarn:0 dfail:0 fail:0 skip:0
fi-hsw-4770 total:108 pass:99 dwarn:0 dfail:0 fail:0 skip:8
fi-hsw-4770r total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:64 time:261s
fi-ivb-3520m total:108 pass:94 dwarn:0 dfail:0 fail:1 skip:12
fi-ivb-3770 total:1 pass:0 dwarn:0 dfail:0 fail:0 skip:0
fi-kbl-7560u total:3 pass:2 dwarn:0 dfail:0 fail:0 skip:0
fi-kbl-7567u total:3 pass:2 dwarn:0 dfail:0 fail:0 skip:0
fi-kbl-r total:3 pass:2 dwarn:0 dfail:0 fail:0 skip:0
fi-pnv-d510 total:216 pass:153 dwarn:14 dfail:1 fail:0 skip:47
fi-skl-6260u total:3 pass:2 dwarn:0 dfail:0 fail:0 skip:0
fi-skl-6600u total:3 pass:2 dwarn:0 dfail:0 fail:0 skip:0
fi-skl-6700hq total:1 pass:0 dwarn:0 dfail:0 fail:0 skip:0
fi-snb-2600 total:3 pass:2 dwarn:0 dfail:0 fail:0 skip:0
Blacklisted hosts:
fi-glk-dsi total:108 pass:95 dwarn:0 dfail:0 fail:0 skip:12
fi-bdw-gvtdvm failed to collect. IGT log at Patchwork_7415/fi-bdw-gvtdvm/igt.log
fi-byt-n2820 failed to collect. IGT log at Patchwork_7415/fi-byt-n2820/igt.log
fi-ilk-650 failed to collect. IGT log at Patchwork_7415/fi-ilk-650/igt.log
fi-kbl-7500u failed to collect. IGT log at Patchwork_7415/fi-kbl-7500u/igt.log
fi-skl-gvtdvm failed to collect. IGT log at Patchwork_7415/fi-skl-gvtdvm/igt.log
fi-snb-2520m failed to connect after reboot
0d0fe916f52ad8f05dddab384ae7c90bb62ebac4 drm-tip: 2017y-12m-05d-14h-52m-17s UTC integration manifest
e6a505e6edf8 intel/atomic: Stop updating legacy fb parameters
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7415/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] intel/atomic: Stop updating legacy fb parameters
@ 2017-12-07 14:32 Daniel Vetter
2017-12-07 14:49 ` Maarten Lankhorst
0 siblings, 1 reply; 7+ messages in thread
From: Daniel Vetter @ 2017-12-07 14:32 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter, Paulo Zanoni, Daniel Vetter
Even fbc isn't using this stuff anymore, so time to remove it.
Cleaning up one small piece of the atomic conversion cruft at the time
...
Quick explanation on why the plane->fb assignment is ok to delete: The
core code takes care of the refcounting and legacy ->fb pointer
updating, but drivers are allowed to update it ahead of time. Most
legacy modeset drivers did that as part of their set_config callback
(since that's how the legacy/crtc helpers worked). In i915 we only
need that to make the fbc code happy.
v2: don't nuke the assignement of intel_crtc->config, I accidentally
set CI ablaze :-) Spotted by Maarten. And better explain why nuking
the ->fb assignement shouldn't set off alarm bells.
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 31 +++----------------------------
1 file changed, 3 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1f7e312d0d0d..4614c7f1eec5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10967,31 +10967,6 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
return ret;
}
-static void
-intel_modeset_update_crtc_state(struct drm_atomic_state *state)
-{
- struct drm_crtc *crtc;
- struct drm_crtc_state *new_crtc_state;
- int i;
-
- /* Double check state. */
- for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
- to_intel_crtc(crtc)->config = to_intel_crtc_state(new_crtc_state);
-
- /*
- * Update legacy state to satisfy fbc code. This can
- * be removed when fbc uses the atomic state.
- */
- if (drm_atomic_get_existing_plane_state(state, crtc->primary)) {
- struct drm_plane_state *plane_state = crtc->primary->state;
-
- crtc->primary->fb = plane_state->fb;
- crtc->x = plane_state->src_x >> 16;
- crtc->y = plane_state->src_y >> 16;
- }
- }
-}
-
static bool intel_fuzzy_clock_check(int clock1, int clock2)
{
int diff;
@@ -12364,9 +12339,9 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
}
}
- /* Only after disabling all output pipelines that will be changed can we
- * update the the output configuration. */
- intel_modeset_update_crtc_state(state);
+ /* FIXME: Eventually get rid of our intel_crtc->config pointer */
+ for_each_new_crtc_in_state(state, crtc, new_crtc_state, i)
+ to_intel_crtc(crtc)->config = to_intel_crtc_state(new_crtc_state);
if (intel_state->modeset) {
drm_atomic_helper_update_legacy_modeset_state(state->dev, state);
--
2.15.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] intel/atomic: Stop updating legacy fb parameters
2017-12-07 14:32 [PATCH] " Daniel Vetter
@ 2017-12-07 14:49 ` Maarten Lankhorst
2017-12-08 10:21 ` Daniel Vetter
0 siblings, 1 reply; 7+ messages in thread
From: Maarten Lankhorst @ 2017-12-07 14:49 UTC (permalink / raw)
To: Daniel Vetter, Intel Graphics Development; +Cc: Daniel Vetter, Paulo Zanoni
Op 07-12-17 om 15:32 schreef Daniel Vetter:
> Even fbc isn't using this stuff anymore, so time to remove it.
>
> Cleaning up one small piece of the atomic conversion cruft at the time
> ...
>
> Quick explanation on why the plane->fb assignment is ok to delete: The
> core code takes care of the refcounting and legacy ->fb pointer
> updating, but drivers are allowed to update it ahead of time. Most
> legacy modeset drivers did that as part of their set_config callback
> (since that's how the legacy/crtc helpers worked). In i915 we only
> need that to make the fbc code happy.
>
> v2: don't nuke the assignement of intel_crtc->config, I accidentally
> set CI ablaze :-) Spotted by Maarten. And better explain why nuking
> the ->fb assignement shouldn't set off alarm bells.
>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 31 +++----------------------------
> 1 file changed, 3 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1f7e312d0d0d..4614c7f1eec5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10967,31 +10967,6 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
> return ret;
> }
>
> -static void
> -intel_modeset_update_crtc_state(struct drm_atomic_state *state)
> -{
> - struct drm_crtc *crtc;
> - struct drm_crtc_state *new_crtc_state;
> - int i;
> -
> - /* Double check state. */
> - for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
> - to_intel_crtc(crtc)->config = to_intel_crtc_state(new_crtc_state);
> -
> - /*
> - * Update legacy state to satisfy fbc code. This can
> - * be removed when fbc uses the atomic state.
> - */
> - if (drm_atomic_get_existing_plane_state(state, crtc->primary)) {
> - struct drm_plane_state *plane_state = crtc->primary->state;
> -
> - crtc->primary->fb = plane_state->fb;
> - crtc->x = plane_state->src_x >> 16;
> - crtc->y = plane_state->src_y >> 16;
> - }
> - }
> -}
> -
> static bool intel_fuzzy_clock_check(int clock1, int clock2)
> {
> int diff;
> @@ -12364,9 +12339,9 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
> }
> }
>
> - /* Only after disabling all output pipelines that will be changed can we
> - * update the the output configuration. */
> - intel_modeset_update_crtc_state(state);
> + /* FIXME: Eventually get rid of our intel_crtc->config pointer */
> + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i)
> + to_intel_crtc(crtc)->config = to_intel_crtc_state(new_crtc_state);
>
> if (intel_state->modeset) {
> drm_atomic_helper_update_legacy_modeset_state(state->dev, state);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] intel/atomic: Stop updating legacy fb parameters
2017-12-07 14:49 ` Maarten Lankhorst
@ 2017-12-08 10:21 ` Daniel Vetter
0 siblings, 0 replies; 7+ messages in thread
From: Daniel Vetter @ 2017-12-08 10:21 UTC (permalink / raw)
To: Maarten Lankhorst
Cc: Daniel Vetter, Intel Graphics Development, Paulo Zanoni,
Daniel Vetter
On Thu, Dec 07, 2017 at 03:49:35PM +0100, Maarten Lankhorst wrote:
> Op 07-12-17 om 15:32 schreef Daniel Vetter:
> > Even fbc isn't using this stuff anymore, so time to remove it.
> >
> > Cleaning up one small piece of the atomic conversion cruft at the time
> > ...
> >
> > Quick explanation on why the plane->fb assignment is ok to delete: The
> > core code takes care of the refcounting and legacy ->fb pointer
> > updating, but drivers are allowed to update it ahead of time. Most
> > legacy modeset drivers did that as part of their set_config callback
> > (since that's how the legacy/crtc helpers worked). In i915 we only
> > need that to make the fbc code happy.
> >
> > v2: don't nuke the assignement of intel_crtc->config, I accidentally
> > set CI ablaze :-) Spotted by Maarten. And better explain why nuking
> > the ->fb assignement shouldn't set off alarm bells.
> >
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Applied, thanks for the review.
-Daniel
>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 31 +++----------------------------
> > 1 file changed, 3 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 1f7e312d0d0d..4614c7f1eec5 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -10967,31 +10967,6 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
> > return ret;
> > }
> >
> > -static void
> > -intel_modeset_update_crtc_state(struct drm_atomic_state *state)
> > -{
> > - struct drm_crtc *crtc;
> > - struct drm_crtc_state *new_crtc_state;
> > - int i;
> > -
> > - /* Double check state. */
> > - for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
> > - to_intel_crtc(crtc)->config = to_intel_crtc_state(new_crtc_state);
> > -
> > - /*
> > - * Update legacy state to satisfy fbc code. This can
> > - * be removed when fbc uses the atomic state.
> > - */
> > - if (drm_atomic_get_existing_plane_state(state, crtc->primary)) {
> > - struct drm_plane_state *plane_state = crtc->primary->state;
> > -
> > - crtc->primary->fb = plane_state->fb;
> > - crtc->x = plane_state->src_x >> 16;
> > - crtc->y = plane_state->src_y >> 16;
> > - }
> > - }
> > -}
> > -
> > static bool intel_fuzzy_clock_check(int clock1, int clock2)
> > {
> > int diff;
> > @@ -12364,9 +12339,9 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
> > }
> > }
> >
> > - /* Only after disabling all output pipelines that will be changed can we
> > - * update the the output configuration. */
> > - intel_modeset_update_crtc_state(state);
> > + /* FIXME: Eventually get rid of our intel_crtc->config pointer */
> > + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i)
> > + to_intel_crtc(crtc)->config = to_intel_crtc_state(new_crtc_state);
> >
> > if (intel_state->modeset) {
> > drm_atomic_helper_update_legacy_modeset_state(state->dev, state);
>
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-12-08 10:21 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-05 17:00 [PATCH] intel/atomic: Stop updating legacy fb parameters Daniel Vetter
2017-12-05 17:40 ` Ville Syrjälä
2017-12-06 10:31 ` Maarten Lankhorst
2017-12-05 18:10 ` ✗ Fi.CI.BAT: failure for " Patchwork
-- strict thread matches above, loose matches on Subject: below --
2017-12-07 14:32 [PATCH] " Daniel Vetter
2017-12-07 14:49 ` Maarten Lankhorst
2017-12-08 10:21 ` Daniel Vetter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.