[PATCH] intel/atomic: Stop updating legacy fb parameters

[PATCH] intel/atomic: Stop updating legacy fb parameters

[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

Re: [PATCH] intel/atomic: Stop updating legacy fb parameters

[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

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for intel/atomic: Stop updating legacy fb parameters

[Patchwork]

== 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

Re: [PATCH] intel/atomic: Stop updating legacy fb parameters

[Maarten Lankhorst]

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

[PATCH] intel/atomic: Stop updating legacy fb parameters

[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

Re: [PATCH] intel/atomic: Stop updating legacy fb parameters

[Maarten Lankhorst]

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

Re: [PATCH] intel/atomic: Stop updating legacy fb parameters

[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