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)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
