diff mbox series

[04/14] drm/i915: Add aliases for uapi and hw to crtc_state

Message ID 20191017132105.15528-4-maarten.lankhorst@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [01/14] drm/i915: Rework watermark readout to use plane api | expand

Commit Message

Maarten Lankhorst Oct. 17, 2019, 1:20 p.m. UTC
Prepare to split up hw and uapi machinally, by adding a uapi and
hw alias. We will remove the base in a bit. This is a split from the
original uapi/hw patch, which did it all in one go.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_atomic.c   |  8 ++++--
 drivers/gpu/drm/i915/display/intel_display.c  |  2 ++
 drivers/gpu/drm/i915/display/intel_display.h  |  6 ++---
 .../drm/i915/display/intel_display_types.h    | 27 ++++++++++++++++++-
 4 files changed, 37 insertions(+), 6 deletions(-)

Comments

Ville Syrjälä Oct. 18, 2019, 10:36 a.m. UTC | #1
On Thu, Oct 17, 2019 at 03:20:55PM +0200, Maarten Lankhorst wrote:
> Prepare to split up hw and uapi machinally, by adding a uapi and
> hw alias. We will remove the base in a bit. This is a split from the
> original uapi/hw patch, which did it all in one go.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_atomic.c   |  8 ++++--
>  drivers/gpu/drm/i915/display/intel_display.c  |  2 ++
>  drivers/gpu/drm/i915/display/intel_display.h  |  6 ++---
>  .../drm/i915/display/intel_display_types.h    | 27 ++++++++++++++++++-
>  4 files changed, 37 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c
> index e6cb85d41c8d..2cdc92897abd 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> @@ -186,9 +186,10 @@ intel_digital_connector_duplicate_state(struct drm_connector *connector)
>  struct drm_crtc_state *
>  intel_crtc_duplicate_state(struct drm_crtc *crtc)
>  {
> +	const struct intel_crtc_state *old_crtc_state = to_intel_crtc_state(crtc->state);
>  	struct intel_crtc_state *crtc_state;
>  
> -	crtc_state = kmemdup(crtc->state, sizeof(*crtc_state), GFP_KERNEL);
> +	crtc_state = kmemdup(old_crtc_state, sizeof(*crtc_state), GFP_KERNEL);
>  	if (!crtc_state)
>  		return NULL;
>  
> @@ -219,7 +220,10 @@ void
>  intel_crtc_destroy_state(struct drm_crtc *crtc,
>  			 struct drm_crtc_state *state)
>  {
> -	drm_atomic_helper_crtc_destroy_state(crtc, state);
> +	struct intel_crtc_state *crtc_state = to_intel_crtc_state(state);
> +
> +	__drm_atomic_helper_crtc_destroy_state(&crtc_state->base);
> +	kfree(crtc_state);
>  }
>  
>  static void intel_atomic_setup_scaler(struct intel_crtc_scaler_state *scaler_state,
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 5632e13d458d..fa0abfdff2ae 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -12282,6 +12282,8 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
>  
>  	/* Keep base drm_crtc_state intact, only clear our extended struct */
>  	BUILD_BUG_ON(offsetof(struct intel_crtc_state, base));
> +	BUILD_BUG_ON(offsetof(struct intel_crtc_state, uapi));
> +	BUILD_BUG_ON(offsetof(struct intel_crtc_state, hw));
>  	memcpy(&crtc_state->base + 1, &saved_state->base + 1,
>  	       sizeof(*crtc_state) - sizeof(crtc_state->base));
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> index 90807603987c..9b53f65386b5 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -444,10 +444,10 @@ enum phy_fia {
>  #define intel_atomic_crtc_state_for_each_plane_state( \
>  		  plane, plane_state, \
>  		  crtc_state) \
> -	for_each_intel_plane_mask(((crtc_state)->base.state->dev), (plane), \
> -				((crtc_state)->base.plane_mask)) \
> +	for_each_intel_plane_mask(((crtc_state)->uapi.state->dev), (plane), \
> +				((crtc_state)->uapi.plane_mask)) \
>  		for_each_if ((plane_state = \
> -			      to_intel_plane_state(__drm_atomic_get_current_plane_state((crtc_state)->base.state, &plane->base))))
> +			      to_intel_plane_state(__drm_atomic_get_current_plane_state((crtc_state)->uapi.state, &plane->base))))
>  
>  void intel_link_compute_m_n(u16 bpp, int nlanes,
>  			    int pixel_clock, int link_clock,
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 244e881474fb..4d85ea5832d7 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -749,7 +749,32 @@ enum intel_output_format {
>  };
>  
>  struct intel_crtc_state {
> +	union {
>  	struct drm_crtc_state base;
> +	/*
> +	 * uapi (drm) state. This is the software state shown to userspace.
> +	 * In particular, the following members are used for bookkeeping:
> +	 * - crtc
> +	 * - state
> +	 * - *_changed
> +	 * - event
> +	 * - commit
> +	 * - mode_blob
> +	 */
> +	struct drm_crtc_state uapi;
> +
> +	/*
> +	 * actual hardware state, the state we program to the hardware.
> +	 * The following members are used to verify the hardware state:
> +	 * - enable
> +	 * - active
> +	 * - mode / adjusted_mode
> +	 * - color property blobs.
> +	 *
> +	 * During initial hw readout, they need to be copied to uapi.
> +	 */
> +	struct drm_crtc_state hw;
> +	};

This part confuses me. Can't we just do

- struct drm_crtc_state base;
+ struct drm_crtc_state uapi;
+ struct {
+	...
+ } base;

?

Or if we somehow need 'base' to remain a drm_crtc_state for a bit then
just start with

+ struct drm_crtc_state uapi;
  struct drm_crtc_state base;

?


>  
>  	/**
>  	 * quirks - bitfield with hw state readout quirks
> @@ -1094,7 +1119,7 @@ struct cxsr_latency {
>  
>  #define to_intel_atomic_state(x) container_of(x, struct intel_atomic_state, base)
>  #define to_intel_crtc(x) container_of(x, struct intel_crtc, base)
> -#define to_intel_crtc_state(x) container_of(x, struct intel_crtc_state, base)
> +#define to_intel_crtc_state(x) container_of(x, struct intel_crtc_state, uapi)
>  #define to_intel_connector(x) container_of(x, struct intel_connector, base)
>  #define to_intel_encoder(x) container_of(x, struct intel_encoder, base)
>  #define to_intel_framebuffer(x) container_of(x, struct intel_framebuffer, base)
> -- 
> 2.23.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Maarten Lankhorst Oct. 18, 2019, 12:09 p.m. UTC | #2
Op 18-10-2019 om 12:36 schreef Ville Syrjälä:
> On Thu, Oct 17, 2019 at 03:20:55PM +0200, Maarten Lankhorst wrote:
>> Prepare to split up hw and uapi machinally, by adding a uapi and
>> hw alias. We will remove the base in a bit. This is a split from the
>> original uapi/hw patch, which did it all in one go.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_atomic.c   |  8 ++++--
>>  drivers/gpu/drm/i915/display/intel_display.c  |  2 ++
>>  drivers/gpu/drm/i915/display/intel_display.h  |  6 ++---
>>  .../drm/i915/display/intel_display_types.h    | 27 ++++++++++++++++++-
>>  4 files changed, 37 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c
>> index e6cb85d41c8d..2cdc92897abd 100644
>> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
>> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
>> @@ -186,9 +186,10 @@ intel_digital_connector_duplicate_state(struct drm_connector *connector)
>>  struct drm_crtc_state *
>>  intel_crtc_duplicate_state(struct drm_crtc *crtc)
>>  {
>> +	const struct intel_crtc_state *old_crtc_state = to_intel_crtc_state(crtc->state);
>>  	struct intel_crtc_state *crtc_state;
>>  
>> -	crtc_state = kmemdup(crtc->state, sizeof(*crtc_state), GFP_KERNEL);
>> +	crtc_state = kmemdup(old_crtc_state, sizeof(*crtc_state), GFP_KERNEL);
>>  	if (!crtc_state)
>>  		return NULL;
>>  
>> @@ -219,7 +220,10 @@ void
>>  intel_crtc_destroy_state(struct drm_crtc *crtc,
>>  			 struct drm_crtc_state *state)
>>  {
>> -	drm_atomic_helper_crtc_destroy_state(crtc, state);
>> +	struct intel_crtc_state *crtc_state = to_intel_crtc_state(state);
>> +
>> +	__drm_atomic_helper_crtc_destroy_state(&crtc_state->base);
>> +	kfree(crtc_state);
>>  }
>>  
>>  static void intel_atomic_setup_scaler(struct intel_crtc_scaler_state *scaler_state,
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index 5632e13d458d..fa0abfdff2ae 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -12282,6 +12282,8 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
>>  
>>  	/* Keep base drm_crtc_state intact, only clear our extended struct */
>>  	BUILD_BUG_ON(offsetof(struct intel_crtc_state, base));
>> +	BUILD_BUG_ON(offsetof(struct intel_crtc_state, uapi));
>> +	BUILD_BUG_ON(offsetof(struct intel_crtc_state, hw));
>>  	memcpy(&crtc_state->base + 1, &saved_state->base + 1,
>>  	       sizeof(*crtc_state) - sizeof(crtc_state->base));
>>  
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
>> index 90807603987c..9b53f65386b5 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display.h
>> @@ -444,10 +444,10 @@ enum phy_fia {
>>  #define intel_atomic_crtc_state_for_each_plane_state( \
>>  		  plane, plane_state, \
>>  		  crtc_state) \
>> -	for_each_intel_plane_mask(((crtc_state)->base.state->dev), (plane), \
>> -				((crtc_state)->base.plane_mask)) \
>> +	for_each_intel_plane_mask(((crtc_state)->uapi.state->dev), (plane), \
>> +				((crtc_state)->uapi.plane_mask)) \
>>  		for_each_if ((plane_state = \
>> -			      to_intel_plane_state(__drm_atomic_get_current_plane_state((crtc_state)->base.state, &plane->base))))
>> +			      to_intel_plane_state(__drm_atomic_get_current_plane_state((crtc_state)->uapi.state, &plane->base))))
>>  
>>  void intel_link_compute_m_n(u16 bpp, int nlanes,
>>  			    int pixel_clock, int link_clock,
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
>> index 244e881474fb..4d85ea5832d7 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> @@ -749,7 +749,32 @@ enum intel_output_format {
>>  };
>>  
>>  struct intel_crtc_state {
>> +	union {
>>  	struct drm_crtc_state base;
>> +	/*
>> +	 * uapi (drm) state. This is the software state shown to userspace.
>> +	 * In particular, the following members are used for bookkeeping:
>> +	 * - crtc
>> +	 * - state
>> +	 * - *_changed
>> +	 * - event
>> +	 * - commit
>> +	 * - mode_blob
>> +	 */
>> +	struct drm_crtc_state uapi;
>> +
>> +	/*
>> +	 * actual hardware state, the state we program to the hardware.
>> +	 * The following members are used to verify the hardware state:
>> +	 * - enable
>> +	 * - active
>> +	 * - mode / adjusted_mode
>> +	 * - color property blobs.
>> +	 *
>> +	 * During initial hw readout, they need to be copied to uapi.
>> +	 */
>> +	struct drm_crtc_state hw;
>> +	};
> This part confuses me. Can't we just do
>
> - struct drm_crtc_state base;
> + struct drm_crtc_state uapi;
> + struct {
> +	...
> + } base;
>
> ?

This is basically union { struct drm_crtc_state uapi,base,hw; }; Making all 3 aliases until patch 8/14, which kills off base and splits off hw state.

We keep it as an union because base and uapi temporarily have to be kept in sync, while we split to uapi and hw. So it's easy to keep hw in sync as well.

In the end it doesn't matter whether we split off hw before or after. The diff between before and after is the same.

So does it look ok now? I can't keep reworking patches. I can merge 8/14 to this patch, and remove the union between base and uapi as patch 8/14.
Ville Syrjälä Oct. 18, 2019, 1:09 p.m. UTC | #3
On Fri, Oct 18, 2019 at 02:09:17PM +0200, Maarten Lankhorst wrote:
> Op 18-10-2019 om 12:36 schreef Ville Syrjälä:
> > On Thu, Oct 17, 2019 at 03:20:55PM +0200, Maarten Lankhorst wrote:
> >> Prepare to split up hw and uapi machinally, by adding a uapi and
> >> hw alias. We will remove the base in a bit. This is a split from the
> >> original uapi/hw patch, which did it all in one go.
> >>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/display/intel_atomic.c   |  8 ++++--
> >>  drivers/gpu/drm/i915/display/intel_display.c  |  2 ++
> >>  drivers/gpu/drm/i915/display/intel_display.h  |  6 ++---
> >>  .../drm/i915/display/intel_display_types.h    | 27 ++++++++++++++++++-
> >>  4 files changed, 37 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c
> >> index e6cb85d41c8d..2cdc92897abd 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> >> @@ -186,9 +186,10 @@ intel_digital_connector_duplicate_state(struct drm_connector *connector)
> >>  struct drm_crtc_state *
> >>  intel_crtc_duplicate_state(struct drm_crtc *crtc)
> >>  {
> >> +	const struct intel_crtc_state *old_crtc_state = to_intel_crtc_state(crtc->state);
> >>  	struct intel_crtc_state *crtc_state;
> >>  
> >> -	crtc_state = kmemdup(crtc->state, sizeof(*crtc_state), GFP_KERNEL);
> >> +	crtc_state = kmemdup(old_crtc_state, sizeof(*crtc_state), GFP_KERNEL);
> >>  	if (!crtc_state)
> >>  		return NULL;
> >>  
> >> @@ -219,7 +220,10 @@ void
> >>  intel_crtc_destroy_state(struct drm_crtc *crtc,
> >>  			 struct drm_crtc_state *state)
> >>  {
> >> -	drm_atomic_helper_crtc_destroy_state(crtc, state);
> >> +	struct intel_crtc_state *crtc_state = to_intel_crtc_state(state);
> >> +
> >> +	__drm_atomic_helper_crtc_destroy_state(&crtc_state->base);
> >> +	kfree(crtc_state);
> >>  }
> >>  
> >>  static void intel_atomic_setup_scaler(struct intel_crtc_scaler_state *scaler_state,
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> >> index 5632e13d458d..fa0abfdff2ae 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> >> @@ -12282,6 +12282,8 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
> >>  
> >>  	/* Keep base drm_crtc_state intact, only clear our extended struct */
> >>  	BUILD_BUG_ON(offsetof(struct intel_crtc_state, base));
> >> +	BUILD_BUG_ON(offsetof(struct intel_crtc_state, uapi));
> >> +	BUILD_BUG_ON(offsetof(struct intel_crtc_state, hw));
> >>  	memcpy(&crtc_state->base + 1, &saved_state->base + 1,
> >>  	       sizeof(*crtc_state) - sizeof(crtc_state->base));
> >>  
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> >> index 90807603987c..9b53f65386b5 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display.h
> >> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> >> @@ -444,10 +444,10 @@ enum phy_fia {
> >>  #define intel_atomic_crtc_state_for_each_plane_state( \
> >>  		  plane, plane_state, \
> >>  		  crtc_state) \
> >> -	for_each_intel_plane_mask(((crtc_state)->base.state->dev), (plane), \
> >> -				((crtc_state)->base.plane_mask)) \
> >> +	for_each_intel_plane_mask(((crtc_state)->uapi.state->dev), (plane), \
> >> +				((crtc_state)->uapi.plane_mask)) \
> >>  		for_each_if ((plane_state = \
> >> -			      to_intel_plane_state(__drm_atomic_get_current_plane_state((crtc_state)->base.state, &plane->base))))
> >> +			      to_intel_plane_state(__drm_atomic_get_current_plane_state((crtc_state)->uapi.state, &plane->base))))
> >>  
> >>  void intel_link_compute_m_n(u16 bpp, int nlanes,
> >>  			    int pixel_clock, int link_clock,
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> >> index 244e881474fb..4d85ea5832d7 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> >> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> >> @@ -749,7 +749,32 @@ enum intel_output_format {
> >>  };
> >>  
> >>  struct intel_crtc_state {
> >> +	union {
> >>  	struct drm_crtc_state base;
> >> +	/*
> >> +	 * uapi (drm) state. This is the software state shown to userspace.
> >> +	 * In particular, the following members are used for bookkeeping:
> >> +	 * - crtc
> >> +	 * - state
> >> +	 * - *_changed
> >> +	 * - event
> >> +	 * - commit
> >> +	 * - mode_blob
> >> +	 */
> >> +	struct drm_crtc_state uapi;
> >> +
> >> +	/*
> >> +	 * actual hardware state, the state we program to the hardware.
> >> +	 * The following members are used to verify the hardware state:
> >> +	 * - enable
> >> +	 * - active
> >> +	 * - mode / adjusted_mode
> >> +	 * - color property blobs.
> >> +	 *
> >> +	 * During initial hw readout, they need to be copied to uapi.
> >> +	 */
> >> +	struct drm_crtc_state hw;
> >> +	};
> > This part confuses me. Can't we just do
> >
> > - struct drm_crtc_state base;
> > + struct drm_crtc_state uapi;
> > + struct {
> > +	...
> > + } base;
> >
> > ?
> 
> This is basically union { struct drm_crtc_state uapi,base,hw; }; Making all 3 aliases until patch 8/14, which kills off base and splits off hw state.
> 
> We keep it as an union because base and uapi temporarily have to be kept in sync, while we split to uapi and hw. So it's easy to keep hw in sync as well.
> 
> In the end it doesn't matter whether we split off hw before or after. The diff between before and after is the same.

Sure but the individual patches are very hard to review. Eg. I can see
a bunch of stuff getting converted from base to hw, but since .base
still exists I have no way of knowing whether the patch really changed
all of them or just some of them.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c
index e6cb85d41c8d..2cdc92897abd 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -186,9 +186,10 @@  intel_digital_connector_duplicate_state(struct drm_connector *connector)
 struct drm_crtc_state *
 intel_crtc_duplicate_state(struct drm_crtc *crtc)
 {
+	const struct intel_crtc_state *old_crtc_state = to_intel_crtc_state(crtc->state);
 	struct intel_crtc_state *crtc_state;
 
-	crtc_state = kmemdup(crtc->state, sizeof(*crtc_state), GFP_KERNEL);
+	crtc_state = kmemdup(old_crtc_state, sizeof(*crtc_state), GFP_KERNEL);
 	if (!crtc_state)
 		return NULL;
 
@@ -219,7 +220,10 @@  void
 intel_crtc_destroy_state(struct drm_crtc *crtc,
 			 struct drm_crtc_state *state)
 {
-	drm_atomic_helper_crtc_destroy_state(crtc, state);
+	struct intel_crtc_state *crtc_state = to_intel_crtc_state(state);
+
+	__drm_atomic_helper_crtc_destroy_state(&crtc_state->base);
+	kfree(crtc_state);
 }
 
 static void intel_atomic_setup_scaler(struct intel_crtc_scaler_state *scaler_state,
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 5632e13d458d..fa0abfdff2ae 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -12282,6 +12282,8 @@  clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
 
 	/* Keep base drm_crtc_state intact, only clear our extended struct */
 	BUILD_BUG_ON(offsetof(struct intel_crtc_state, base));
+	BUILD_BUG_ON(offsetof(struct intel_crtc_state, uapi));
+	BUILD_BUG_ON(offsetof(struct intel_crtc_state, hw));
 	memcpy(&crtc_state->base + 1, &saved_state->base + 1,
 	       sizeof(*crtc_state) - sizeof(crtc_state->base));
 
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index 90807603987c..9b53f65386b5 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -444,10 +444,10 @@  enum phy_fia {
 #define intel_atomic_crtc_state_for_each_plane_state( \
 		  plane, plane_state, \
 		  crtc_state) \
-	for_each_intel_plane_mask(((crtc_state)->base.state->dev), (plane), \
-				((crtc_state)->base.plane_mask)) \
+	for_each_intel_plane_mask(((crtc_state)->uapi.state->dev), (plane), \
+				((crtc_state)->uapi.plane_mask)) \
 		for_each_if ((plane_state = \
-			      to_intel_plane_state(__drm_atomic_get_current_plane_state((crtc_state)->base.state, &plane->base))))
+			      to_intel_plane_state(__drm_atomic_get_current_plane_state((crtc_state)->uapi.state, &plane->base))))
 
 void intel_link_compute_m_n(u16 bpp, int nlanes,
 			    int pixel_clock, int link_clock,
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 244e881474fb..4d85ea5832d7 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -749,7 +749,32 @@  enum intel_output_format {
 };
 
 struct intel_crtc_state {
+	union {
 	struct drm_crtc_state base;
+	/*
+	 * uapi (drm) state. This is the software state shown to userspace.
+	 * In particular, the following members are used for bookkeeping:
+	 * - crtc
+	 * - state
+	 * - *_changed
+	 * - event
+	 * - commit
+	 * - mode_blob
+	 */
+	struct drm_crtc_state uapi;
+
+	/*
+	 * actual hardware state, the state we program to the hardware.
+	 * The following members are used to verify the hardware state:
+	 * - enable
+	 * - active
+	 * - mode / adjusted_mode
+	 * - color property blobs.
+	 *
+	 * During initial hw readout, they need to be copied to uapi.
+	 */
+	struct drm_crtc_state hw;
+	};
 
 	/**
 	 * quirks - bitfield with hw state readout quirks
@@ -1094,7 +1119,7 @@  struct cxsr_latency {
 
 #define to_intel_atomic_state(x) container_of(x, struct intel_atomic_state, base)
 #define to_intel_crtc(x) container_of(x, struct intel_crtc, base)
-#define to_intel_crtc_state(x) container_of(x, struct intel_crtc_state, base)
+#define to_intel_crtc_state(x) container_of(x, struct intel_crtc_state, uapi)
 #define to_intel_connector(x) container_of(x, struct intel_connector, base)
 #define to_intel_encoder(x) container_of(x, struct intel_encoder, base)
 #define to_intel_framebuffer(x) container_of(x, struct intel_framebuffer, base)