diff mbox

[v2] drm/i915: Initialize primary plane src/dst coords when reading hw state

Message ID 1421675507-27529-1-git-send-email-ander.conselvan.de.oliveira@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ander Conselvan de Oliveira Jan. 19, 2015, 1:51 p.m. UTC
Otherwise setting the rotation property will cause the primary plane to
be disabled, caused by having a 0x0 initial value.

v2: Rebase on top of the move to plane helpers.

Cc: stable@vger.kernel.org
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87662
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

Shuang He Jan. 19, 2015, 11:45 p.m. UTC | #1
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 5604
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  353/353              353/353
ILK                                  353/353              353/353
SNB                                  400/422              400/422
IVB                                  487/487              487/487
BYT                                  296/296              296/296
HSW              +21                 487/508              508/508
BDW                                  401/402              401/402
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 HSW  igt_kms_cursor_crc_cursor-size-change      NSPT(1, M19)TIMEOUT(1, M40)PASS(7, M20M19M40)      PASS(1, M20)
 HSW  igt_kms_fence_pin_leak      NSPT(1, M19)DMESG_WARN(1, M40)PASS(7, M20M19M40)      PASS(1, M20)
 HSW  igt_kms_mmio_vs_cs_flip_setcrtc_vs_cs_flip      NSPT(1, M19)TIMEOUT(1, M40)PASS(7, M20M19M40)      PASS(1, M20)
 HSW  igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip      NSPT(1, M19)TIMEOUT(1, M40)PASS(7, M20M19M40)      PASS(1, M20)
 HSW  igt_pm_lpsp_non-edp      NSPT(1, M19)PASS(7, M20M19M40)      PASS(1, M20)
 HSW  igt_pm_rpm_cursor      NSPT(1, M19)PASS(7, M20M19M40)      PASS(1, M20)
 HSW  igt_pm_rpm_cursor-dpms      NSPT(1, M19)PASS(7, M20M19M40)      PASS(1, M20)
 HSW  igt_pm_rpm_dpms-mode-unset-non-lpsp      NSPT(1, M19)PASS(7, M20M19M40)      PASS(1, M20)
 HSW  igt_pm_rpm_dpms-non-lpsp      NSPT(1, M19)PASS(7, M20M19M40)      PASS(1, M20)
 HSW  igt_pm_rpm_drm-resources-equal      NSPT(1, M19)PASS(7, M20M19M40)      PASS(1, M20)
 HSW  igt_pm_rpm_fences      NSPT(1, M19)PASS(7, M20M19M40)      PASS(1, M20)
 HSW  igt_pm_rpm_fences-dpms      NSPT(1, M19)PASS(7, M20M19M40)      PASS(1, M20)
 HSW  igt_pm_rpm_gem-execbuf      NSPT(1, M19)PASS(7, M20M19M40)      PASS(1, M20)
 HSW  igt_pm_rpm_gem-mmap-cpu      NSPT(1, M19)PASS(7, M20M19M40)      PASS(1, M20)
 HSW  igt_pm_rpm_gem-mmap-gtt      NSPT(1, M19)PASS(7, M20M19M40)      PASS(1, M20)
 HSW  igt_pm_rpm_gem-pread      NSPT(1, M19)PASS(7, M20M19M40)      PASS(1, M20)
 HSW  igt_pm_rpm_i2c      NSPT(1, M19)PASS(7, M20M19M40)      PASS(1, M20)
 HSW  igt_pm_rpm_modeset-non-lpsp      NSPT(1, M19)PASS(7, M20M19M40)      PASS(1, M20)
 HSW  igt_pm_rpm_modeset-non-lpsp-stress-no-wait      NSPT(1, M19)PASS(7, M20M19M40)      PASS(1, M20)
 HSW  igt_pm_rpm_pci-d3-state      NSPT(1, M19)PASS(7, M20M19M40)      PASS(1, M20)
 HSW  igt_pm_rpm_rte      NSPT(1, M19)PASS(7, M20M19M40)      PASS(1, M20)
Note: You need to pay more attention to line start with '*'
Daniel Vetter Jan. 20, 2015, 9:22 a.m. UTC | #2
On Mon, Jan 19, 2015 at 03:51:47PM +0200, Ander Conselvan de Oliveira wrote:
> Otherwise setting the rotation property will cause the primary plane to
> be disabled, caused by having a 0x0 initial value.
> 
> v2: Rebase on top of the move to plane helpers.
> 
> Cc: stable@vger.kernel.org
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87662
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

Testcase: igt/ksm_plane/*-suspend-*

> ---
>  drivers/gpu/drm/i915/intel_display.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 91d8ada..ed70ca7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13327,6 +13327,23 @@ static bool primary_get_hw_state(struct intel_crtc *crtc)
>  	return I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE;
>  }
>  
> +static void primary_update_size(struct intel_crtc *crtc)
> +{
> +	struct drm_plane_state *state = crtc->base.primary->state;
> +
> +	if (!crtc->primary_enabled)
> +		return;
> +
> +	state->crtc_x = 0;
> +	state->crtc_y = 0;
> +	state->crtc_w = crtc->config.pipe_src_w;
> +	state->crtc_h = crtc->config.pipe_src_h;
> +	state->src_x = 0;
> +	state->src_y = 0;
> +	state->src_w = crtc->config.pipe_src_w << 16;
> +	state->src_h = crtc->config.pipe_src_h << 16;
> +}
> +
>  static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -13337,6 +13354,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  	int i;
>  
>  	for_each_intel_crtc(dev, crtc) {
> +

Spurious hunk.

>  		memset(&crtc->config, 0, sizeof(crtc->config));
>  
>  		crtc->config.quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
> @@ -13346,6 +13364,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  
>  		crtc->base.enabled = crtc->active;
>  		crtc->primary_enabled = primary_get_hw_state(crtc);
> +		primary_update_size(crtc);

I think Ville raised a really good point about the fragility of this and
restoring plane state correctly. I think conceptually it makes more sense
to restore the primary plane state together with the fb in the loop at the
end of intel_modeset_init. Would that still work, or is that too late for
when we change pipe state when sanititizing crtcs?
-Daniel

>  
>  		DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
>  			      crtc->base.base.id,
> -- 
> 1.9.1
>
Ander Conselvan de Oliveira Jan. 20, 2015, 9:32 a.m. UTC | #3
On 01/20/2015 11:22 AM, Daniel Vetter wrote:
> On Mon, Jan 19, 2015 at 03:51:47PM +0200, Ander Conselvan de Oliveira wrote:
>> Otherwise setting the rotation property will cause the primary plane to
>> be disabled, caused by having a 0x0 initial value.
>>
>> v2: Rebase on top of the move to plane helpers.
>>
>> Cc: stable@vger.kernel.org
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87662
>> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
>
> Testcase: igt/ksm_plane/*-suspend-*
>
>> ---
>>   drivers/gpu/drm/i915/intel_display.c | 19 +++++++++++++++++++
>>   1 file changed, 19 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 91d8ada..ed70ca7 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -13327,6 +13327,23 @@ static bool primary_get_hw_state(struct intel_crtc *crtc)
>>   	return I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE;
>>   }
>>
>> +static void primary_update_size(struct intel_crtc *crtc)
>> +{
>> +	struct drm_plane_state *state = crtc->base.primary->state;
>> +
>> +	if (!crtc->primary_enabled)
>> +		return;
>> +
>> +	state->crtc_x = 0;
>> +	state->crtc_y = 0;
>> +	state->crtc_w = crtc->config.pipe_src_w;
>> +	state->crtc_h = crtc->config.pipe_src_h;
>> +	state->src_x = 0;
>> +	state->src_y = 0;
>> +	state->src_w = crtc->config.pipe_src_w << 16;
>> +	state->src_h = crtc->config.pipe_src_h << 16;
>> +}
>> +
>>   static void intel_modeset_readout_hw_state(struct drm_device *dev)
>>   {
>>   	struct drm_i915_private *dev_priv = dev->dev_private;
>> @@ -13337,6 +13354,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>>   	int i;
>>
>>   	for_each_intel_crtc(dev, crtc) {
>> +
>
> Spurious hunk.
>
>>   		memset(&crtc->config, 0, sizeof(crtc->config));
>>
>>   		crtc->config.quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
>> @@ -13346,6 +13364,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>>
>>   		crtc->base.enabled = crtc->active;
>>   		crtc->primary_enabled = primary_get_hw_state(crtc);
>> +		primary_update_size(crtc);
>
> I think Ville raised a really good point about the fragility of this and
> restoring plane state correctly. I think conceptually it makes more sense
> to restore the primary plane state together with the fb in the loop at the
> end of intel_modeset_init. Would that still work, or is that too late for
> when we change pipe state when sanititizing crtcs?

That should work. Actually, Chris sent a patch that did that some time 
ago, and Ville commented that "[he was] thinking that for now these 
would be better placed in intel_modeset_readout_hw_state() where we read 
out the primary plane enabled state as well". [1]

So just to make it complete clear, does the problem of restoring plane 
state correctly supersedes Ville's previous comment?

[1] 
http://permalink.gmane.org/gmane.comp.freedesktop.xorg.drivers.intel/46466

Ander


> -Daniel
>
>>
>>   		DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
>>   			      crtc->base.base.id,
>> --
>> 1.9.1
>>
>

---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
Ander Conselvan de Oliveira Jan. 20, 2015, 9:33 a.m. UTC | #4
On 01/20/2015 11:22 AM, Daniel Vetter wrote:
> On Mon, Jan 19, 2015 at 03:51:47PM +0200, Ander Conselvan de Oliveira wrote:
>> Otherwise setting the rotation property will cause the primary plane to
>> be disabled, caused by having a 0x0 initial value.
>>
>> v2: Rebase on top of the move to plane helpers.
>>
>> Cc: stable@vger.kernel.org
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87662
>> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
>
> Testcase: igt/ksm_plane/*-suspend-*
>
>> ---
>>   drivers/gpu/drm/i915/intel_display.c | 19 +++++++++++++++++++
>>   1 file changed, 19 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 91d8ada..ed70ca7 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -13327,6 +13327,23 @@ static bool primary_get_hw_state(struct intel_crtc *crtc)
>>   	return I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE;
>>   }
>>
>> +static void primary_update_size(struct intel_crtc *crtc)
>> +{
>> +	struct drm_plane_state *state = crtc->base.primary->state;
>> +
>> +	if (!crtc->primary_enabled)
>> +		return;
>> +
>> +	state->crtc_x = 0;
>> +	state->crtc_y = 0;
>> +	state->crtc_w = crtc->config.pipe_src_w;
>> +	state->crtc_h = crtc->config.pipe_src_h;
>> +	state->src_x = 0;
>> +	state->src_y = 0;
>> +	state->src_w = crtc->config.pipe_src_w << 16;
>> +	state->src_h = crtc->config.pipe_src_h << 16;
>> +}
>> +
>>   static void intel_modeset_readout_hw_state(struct drm_device *dev)
>>   {
>>   	struct drm_i915_private *dev_priv = dev->dev_private;
>> @@ -13337,6 +13354,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>>   	int i;
>>
>>   	for_each_intel_crtc(dev, crtc) {
>> +
>
> Spurious hunk.
>
>>   		memset(&crtc->config, 0, sizeof(crtc->config));
>>
>>   		crtc->config.quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
>> @@ -13346,6 +13364,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>>
>>   		crtc->base.enabled = crtc->active;
>>   		crtc->primary_enabled = primary_get_hw_state(crtc);
>> +		primary_update_size(crtc);
>
> I think Ville raised a really good point about the fragility of this and
> restoring plane state correctly. I think conceptually it makes more sense
> to restore the primary plane state together with the fb in the loop at the
> end of intel_modeset_init. Would that still work, or is that too late for
> when we change pipe state when sanititizing crtcs?

That should work. Actually, Chris sent a patch that did that some time 
ago, and Ville commented that "[he was] thinking that for now these 
would be better placed in intel_modeset_readout_hw_state() where we read 
out the primary plane enabled state as well". [1]

So just to make it completely clear, does the problem of restoring plane 
state correctly supersedes Ville's previous comment?

[1] 
http://permalink.gmane.org/gmane.comp.freedesktop.xorg.drivers.intel/46466

Ander


> -Daniel
>
>>
>>   		DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
>>   			      crtc->base.base.id,
>> --
>> 1.9.1
>>
>
Ville Syrjala Jan. 20, 2015, 9:46 a.m. UTC | #5
On Tue, Jan 20, 2015 at 11:33:22AM +0200, Ander Conselvan de Oliveira wrote:
> On 01/20/2015 11:22 AM, Daniel Vetter wrote:
> > On Mon, Jan 19, 2015 at 03:51:47PM +0200, Ander Conselvan de Oliveira wrote:
> >> Otherwise setting the rotation property will cause the primary plane to
> >> be disabled, caused by having a 0x0 initial value.
> >>
> >> v2: Rebase on top of the move to plane helpers.
> >>
> >> Cc: stable@vger.kernel.org
> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87662
> >> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> >
> > Testcase: igt/ksm_plane/*-suspend-*
> >
> >> ---
> >>   drivers/gpu/drm/i915/intel_display.c | 19 +++++++++++++++++++
> >>   1 file changed, 19 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> index 91d8ada..ed70ca7 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -13327,6 +13327,23 @@ static bool primary_get_hw_state(struct intel_crtc *crtc)
> >>   	return I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE;
> >>   }
> >>
> >> +static void primary_update_size(struct intel_crtc *crtc)
> >> +{
> >> +	struct drm_plane_state *state = crtc->base.primary->state;
> >> +
> >> +	if (!crtc->primary_enabled)
> >> +		return;
> >> +
> >> +	state->crtc_x = 0;
> >> +	state->crtc_y = 0;
> >> +	state->crtc_w = crtc->config.pipe_src_w;
> >> +	state->crtc_h = crtc->config.pipe_src_h;
> >> +	state->src_x = 0;
> >> +	state->src_y = 0;
> >> +	state->src_w = crtc->config.pipe_src_w << 16;
> >> +	state->src_h = crtc->config.pipe_src_h << 16;
> >> +}
> >> +
> >>   static void intel_modeset_readout_hw_state(struct drm_device *dev)
> >>   {
> >>   	struct drm_i915_private *dev_priv = dev->dev_private;
> >> @@ -13337,6 +13354,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> >>   	int i;
> >>
> >>   	for_each_intel_crtc(dev, crtc) {
> >> +
> >
> > Spurious hunk.
> >
> >>   		memset(&crtc->config, 0, sizeof(crtc->config));
> >>
> >>   		crtc->config.quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
> >> @@ -13346,6 +13364,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> >>
> >>   		crtc->base.enabled = crtc->active;
> >>   		crtc->primary_enabled = primary_get_hw_state(crtc);
> >> +		primary_update_size(crtc);
> >
> > I think Ville raised a really good point about the fragility of this and
> > restoring plane state correctly. I think conceptually it makes more sense
> > to restore the primary plane state together with the fb in the loop at the
> > end of intel_modeset_init. Would that still work, or is that too late for
> > when we change pipe state when sanititizing crtcs?
> 
> That should work. Actually, Chris sent a patch that did that some time 
> ago, and Ville commented that "[he was] thinking that for now these 
> would be better placed in intel_modeset_readout_hw_state() where we read 
> out the primary plane enabled state as well". [1]
> 
> So just to make it completely clear, does the problem of restoring plane 
> state correctly supersedes Ville's previous comment?

Well, I still think intel_modeset_readout_hw_state() is the right place
for this. We would just need to keep the user state and current state
clearly separated (intel_modeset_readout_hw_state() should not touch the
user state). But if that's too hard to do without massive changes, maybe
we can put it somewhere else in the meantime.

> 
> [1] 
> http://permalink.gmane.org/gmane.comp.freedesktop.xorg.drivers.intel/46466
> 
> Ander
> 
> 
> > -Daniel
> >
> >>
> >>   		DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
> >>   			      crtc->base.base.id,
> >> --
> >> 1.9.1
> >>
> >
Daniel Vetter Jan. 20, 2015, 9:56 a.m. UTC | #6
On Tue, Jan 20, 2015 at 11:46:57AM +0200, Ville Syrjälä wrote:
> On Tue, Jan 20, 2015 at 11:33:22AM +0200, Ander Conselvan de Oliveira wrote:
> > On 01/20/2015 11:22 AM, Daniel Vetter wrote:
> > > On Mon, Jan 19, 2015 at 03:51:47PM +0200, Ander Conselvan de Oliveira wrote:
> > >> @@ -13346,6 +13364,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> > >>
> > >>   		crtc->base.enabled = crtc->active;
> > >>   		crtc->primary_enabled = primary_get_hw_state(crtc);
> > >> +		primary_update_size(crtc);
> > >
> > > I think Ville raised a really good point about the fragility of this and
> > > restoring plane state correctly. I think conceptually it makes more sense
> > > to restore the primary plane state together with the fb in the loop at the
> > > end of intel_modeset_init. Would that still work, or is that too late for
> > > when we change pipe state when sanititizing crtcs?
> > 
> > That should work. Actually, Chris sent a patch that did that some time 
> > ago, and Ville commented that "[he was] thinking that for now these 
> > would be better placed in intel_modeset_readout_hw_state() where we read 
> > out the primary plane enabled state as well". [1]
> > 
> > So just to make it completely clear, does the problem of restoring plane 
> > state correctly supersedes Ville's previous comment?
> 
> Well, I still think intel_modeset_readout_hw_state() is the right place
> for this. We would just need to keep the user state and current state
> clearly separated (intel_modeset_readout_hw_state() should not touch the
> user state). But if that's too hard to do without massive changes, maybe
> we can put it somewhere else in the meantime.

Well most of the plane state readout for the initial config (which this
seems to be a problem with) isn't in that function but in modeset_init.
And we currently have no means to check the requested vs the actual state
for the plane configuration.

Thinking about that we probably don't need that really: The reason for the
modeset config checks is to catch bugs with requested vs. actual state,
since we can't test anything really automatically. But with plane config
changes we can do full functional tests using display CRCs. So from that
pov I don't see a big reason to add all that complexity.

Long term we should try to consolidate the plane state readout a bit
(outside of readout_hw_state perhaps), but for now just adding the code in
the modeset_init loop seems best to me.

>> > [1] 
> > http://permalink.gmane.org/gmane.comp.freedesktop.xorg.drivers.intel/46466

In short I disagree with your comment referenced above.
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 91d8ada..ed70ca7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13327,6 +13327,23 @@  static bool primary_get_hw_state(struct intel_crtc *crtc)
 	return I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE;
 }
 
+static void primary_update_size(struct intel_crtc *crtc)
+{
+	struct drm_plane_state *state = crtc->base.primary->state;
+
+	if (!crtc->primary_enabled)
+		return;
+
+	state->crtc_x = 0;
+	state->crtc_y = 0;
+	state->crtc_w = crtc->config.pipe_src_w;
+	state->crtc_h = crtc->config.pipe_src_h;
+	state->src_x = 0;
+	state->src_y = 0;
+	state->src_w = crtc->config.pipe_src_w << 16;
+	state->src_h = crtc->config.pipe_src_h << 16;
+}
+
 static void intel_modeset_readout_hw_state(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -13337,6 +13354,7 @@  static void intel_modeset_readout_hw_state(struct drm_device *dev)
 	int i;
 
 	for_each_intel_crtc(dev, crtc) {
+
 		memset(&crtc->config, 0, sizeof(crtc->config));
 
 		crtc->config.quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
@@ -13346,6 +13364,7 @@  static void intel_modeset_readout_hw_state(struct drm_device *dev)
 
 		crtc->base.enabled = crtc->active;
 		crtc->primary_enabled = primary_get_hw_state(crtc);
+		primary_update_size(crtc);
 
 		DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
 			      crtc->base.base.id,