[Intel-gfx] [PATCH 08/24] drm/i915: Prepare to split crtc state in uapi and hw state

[Maarten Lankhorst]

Op 08-10-2019 om 19:06 schreef Ville Syrjälä:
> On Fri, Oct 04, 2019 at 01:34:58PM +0200, Maarten Lankhorst wrote:
>> We want to split drm_crtc_state into the user visible state
>> and actual hardware state. To prepare for this, we need some
>> ground rules what should be in each state:
>>
>> In uapi we use:
>> - crtc, *_changed flags, event, commit, state, mode_blob,
>>   (plane/connector/encoder)_mask.
>>
>> In hw state we use what's displayed in hardware:
>> - enable, active, (adjusted) mode, color property blobs.
>>
>> clear_intel_crtc_state and hw readout need to be updated for these rules,
>> which will allow us to enable 2 joined pipes.
> I still have hard time with reading this patch. I still think it
> would be easier to read if we didn't do both the "uapi" and "hw" changes
> at the same time.
>
> step 1.
> 	struct drm_crtc_state uapi;
> 	struct {
> 		// hw state
> 	} base;
>
> step 2. 
> 	s/base/hw/
>
> I think that would make it more obvious which parts of the code are
> looking at which state.

It wouldn't I think, but here's
a dumb change with spatch on this patch.

//+       struct {
//+               bool active, enable;
//+               struct drm_property_blob *degamma_lut, *gamma_lut, *ctm;
//+               struct drm_display_mode mode, adjusted_mode;
//+       } hw;

@@
struct intel_crtc_state *T;
@@
-T->uapi.active
+T->hw.active

@@
struct intel_crtc_state *T;
@@
-T->uapi.enable
+T->hw.enable

@@
struct intel_crtc_state *T;
@@
-T->uapi.degamma_lut
+T->hw.degamma_lut


@@
struct intel_crtc_state *T;
@@
-T->uapi.gamma_lut
+T->hw.gamma_lut

@@
struct intel_crtc_state *T;
@@
-T->uapi.ctm
+T->hw.ctm

@@
struct intel_crtc_state *T;
@@
-T->uapi.mode
+T->hw.mode

@@
struct intel_crtc_state *T;
@@
-T->uapi.adjusted_mode
+T->hw.adjusted_mode

I replaced all the instances where we use the uapi members instead of the hw members explicitly in this patch, and came up with the following diff below.

Only the intel_color readout is potentially incorrect, the 2 explicit uapi uses in intel_display.c are needed.
Didn't fix it because of hw readout, it possibly needs slightly more thought.

Does this satisfy the readability requirements? :)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index ab10c33266bf..cbf4c6e6e661 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -11217,7 +11217,7 @@ int intel_get_load_detect_pipe(struct drm_connector *connector,
 		goto fail;
 	}
 
-	crtc_state->uapi.active = crtc_state->uapi.enable = true;
+	crtc_state->hw.active = crtc_state->hw.enable = true;
 
 	if (!mode)
 		mode = &load_detect_mode;
@@ -13578,7 +13578,7 @@ static int intel_atomic_check(struct drm_device *dev,
 		if (!needs_modeset(new_crtc_state))
 			continue;
 
-		if (!new_crtc_state->uapi.enable) {
+		if (!new_crtc_state->hw.enable) {
 			any_ms = true;
 			continue;
 		}

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 5586891572f8..52712bb9ed15 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1623,7 +1623,7 @@ static void i9xx_read_luts(struct intel_crtc_state *crtc_state)
 	if (!crtc_state->gamma_enable)
 		return;
 
-	crtc_state->uapi.gamma_lut = i9xx_read_lut_8(crtc_state);
+	crtc_state->hw.gamma_lut = i9xx_read_lut_8(crtc_state);
 }
 
 static struct drm_property_blob *
@@ -1673,9 +1673,9 @@ static void i965_read_luts(struct intel_crtc_state *crtc_state)
 		return;
 
 	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
-		crtc_state->uapi.gamma_lut = i9xx_read_lut_8(crtc_state);
+		crtc_state->hw.gamma_lut = i9xx_read_lut_8(crtc_state);
 	else
-		crtc_state->uapi.gamma_lut = i965_read_lut_10p6(crtc_state);
+		crtc_state->hw.gamma_lut = i965_read_lut_10p6(crtc_state);
 }
 
 static struct drm_property_blob *
@@ -1715,7 +1715,7 @@ chv_read_cgm_lut(const struct intel_crtc_state *crtc_state)
 static void chv_read_luts(struct intel_crtc_state *crtc_state)
 {
 	if (crtc_state->cgm_mode & CGM_PIPE_MODE_GAMMA)
-		crtc_state->uapi.gamma_lut = chv_read_cgm_lut(crtc_state);
+		crtc_state->hw.gamma_lut = chv_read_cgm_lut(crtc_state);
 	else
 		i965_read_luts(crtc_state);
 }
@@ -1762,9 +1762,9 @@ static void ilk_read_luts(struct intel_crtc_state *crtc_state)
 		return;
 
 	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
-		crtc_state->uapi.gamma_lut = i9xx_read_lut_8(crtc_state);
+		crtc_state->hw.gamma_lut = i9xx_read_lut_8(crtc_state);
 	else
-		crtc_state->uapi.gamma_lut = ilk_read_lut_10(crtc_state);
+		crtc_state->hw.gamma_lut = ilk_read_lut_10(crtc_state);
 }
 
 static struct drm_property_blob *
@@ -1811,9 +1811,9 @@ static void glk_read_luts(struct intel_crtc_state *crtc_state)
 		return;
 
 	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
-		crtc_state->uapi.gamma_lut = i9xx_read_lut_8(crtc_state);
+		crtc_state->hw.gamma_lut = i9xx_read_lut_8(crtc_state);
 	else
-		crtc_state->uapi.gamma_lut = glk_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));
+		crtc_state->hw.gamma_lut = glk_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));
 }
 
 void intel_color_init(struct intel_crtc *crtc)