[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
...

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

✓ Fi.CI.BAT: success for intel/atomic: Stop updating legacy fb parameters (rev2)

[Patchwork]

== Series Details ==

Series: intel/atomic: Stop updating legacy fb parameters (rev2)
URL   : https://patchwork.freedesktop.org/series/34924/
State : success

== Summary ==

Series 34924v2 intel/atomic: Stop updating legacy fb parameters
https://patchwork.freedesktop.org/api/1.0/series/34924/revisions/2/mbox/

Test debugfs_test:
        Subgroup read_all_entries:
                dmesg-warn -> DMESG-FAIL (fi-elk-e7500) fdo#103989
Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                fail       -> PASS       (fi-gdg-551) fdo#102575
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-c:
                pass       -> INCOMPLETE (fi-hsw-4770) fdo#103375

fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:444s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:385s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:524s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:281s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:504s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:512s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:489s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:475s
fi-elk-e7500     total:224  pass:163  dwarn:14  dfail:1   fail:0   skip:45 
fi-gdg-551       total:288  pass:179  dwarn:1   dfail:0   fail:0   skip:108 time:268s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:543s
fi-hsw-4770      total:246  pass:222  dwarn:0   dfail:0   fail:0   skip:23 
fi-hsw-4770r     total:288  pass:224  dwarn:0   dfail:0   fail:0   skip:64  time:257s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:391s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:489s
fi-ivb-3770      total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:450s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:489s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:525s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:476s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:530s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:591s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:454s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:543s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:562s
fi-skl-6700k     total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:519s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:495s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:453s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:418s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:608s
fi-cnl-y         total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:624s
fi-glk-dsi       total:288  pass:257  dwarn:0   dfail:0   fail:1   skip:30  time:501s
fi-snb-2520m failed to collect. IGT log at Patchwork_7439/fi-snb-2520m/igt.log

bdf9b36fd601c2aa06fb191b31d0bac1197db8d0 drm-tip: 2017y-12m-07d-14h-56m-01s UTC integration manifest
0a74c361c039 intel/atomic: Stop updating legacy fb parameters

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7439/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: warning for intel/atomic: Stop updating legacy fb parameters (rev2)

[Patchwork]

== Series Details ==

Series: intel/atomic: Stop updating legacy fb parameters (rev2)
URL   : https://patchwork.freedesktop.org/series/34924/
State : warning

== Summary ==

Test kms_cursor_crc:
        Subgroup cursor-64x64-suspend:
                skip       -> PASS       (shard-snb) fdo#102365
Test gem_tiled_swapping:
        Subgroup non-threaded:
                incomplete -> PASS       (shard-hsw) fdo#104009
Test perf:
        Subgroup blocking:
                pass       -> FAIL       (shard-hsw) fdo#102252
Test drv_suspend:
        Subgroup forcewake:
                skip       -> PASS       (shard-hsw)
Test drv_module_reload:
        Subgroup basic-reload:
                dmesg-warn -> PASS       (shard-hsw) fdo#102707
Test kms_frontbuffer_tracking:
        Subgroup fbc-rgb565-draw-blt:
                pass       -> SKIP       (shard-snb) fdo#101623
Test kms_busy:
        Subgroup extended-pageflip-hang-oldfb-render-b:
                pass       -> SKIP       (shard-snb)
Test kms_atomic_transition:
        Subgroup 1x-modeset-transitions:
                pass       -> SKIP       (shard-snb)

fdo#102365 https://bugs.freedesktop.org/show_bug.cgi?id=102365
fdo#104009 https://bugs.freedesktop.org/show_bug.cgi?id=104009
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#102707 https://bugs.freedesktop.org/show_bug.cgi?id=102707
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623

shard-hsw        total:2679 pass:1535 dwarn:1   dfail:0   fail:11  skip:1132 time:9490s
shard-snb        total:2679 pass:1305 dwarn:2   dfail:0   fail:11  skip:1361 time:8133s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7439/shards.html
_______________________________________________
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

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

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

[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