Op 29-10-2019 om 19:34 schreef Ville Syrjälä:
> On Tue, Oct 29, 2019 at 08:22:29AM +0100, Maarten Lankhorst wrote:
>> Splitting plane state is easier than splitting crtc_state,
>> before plane check we copy the drm properties to hw so we can
>> do the same in bigjoiner later on.
>>
>> We copy the state after we did all the modeset handling, but fortunately
>> i915 seems to be split correctly and nothing during modeset looks
>> at plane_state.
>>
>> Changes since v1:
>> - Do not clear hw state on duplication.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
>> ---
>> .../gpu/drm/i915/display/intel_atomic_plane.c | 37 ++++++++++++++++++-
>> .../gpu/drm/i915/display/intel_atomic_plane.h | 2 +
>> drivers/gpu/drm/i915/display/intel_display.c | 1 +
>> .../drm/i915/display/intel_display_types.h | 23 +++++++++---
>> 4 files changed, 57 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
>> index ba7d5421f791..75a5004b234e 100644
>> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
>> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
>> @@ -93,6 +93,10 @@ intel_plane_duplicate_state(struct drm_plane *plane)
>> intel_state->vma = NULL;
>> intel_state->flags = 0;
>>
>> + /* add reference to fb */
>> + if (intel_state->hw.fb)
>> + drm_framebuffer_get(intel_state->hw.fb);
>> +
>> return &intel_state->uapi;
>> }
>>
>> @@ -112,6 +116,8 @@ intel_plane_destroy_state(struct drm_plane *plane,
>> WARN_ON(plane_state->vma);
>>
>> __drm_atomic_helper_plane_destroy_state(&plane_state->uapi);
>> + if (plane_state->hw.fb)
>> + drm_framebuffer_put(plane_state->hw.fb);
>> kfree(plane_state);
>> }
>>
>> @@ -176,15 +182,44 @@ bool intel_plane_calc_min_cdclk(struct intel_atomic_state *state,
>> return false;
>> }
>>
>> +static void intel_plane_clear_hw_state(struct intel_plane_state *plane_state)
>> +{
>> + if (plane_state->hw.fb)
>> + drm_framebuffer_put(plane_state->hw.fb);
>> +
>> + memset(&plane_state->hw, 0, sizeof(plane_state->hw));
>> +}
>> +
>> +void intel_plane_copy_uapi_to_hw_state(struct intel_plane_state *plane_state,
>> + const struct intel_plane_state *from_plane_state)
>> +{
>> + intel_plane_clear_hw_state(plane_state);
>> +
>> + plane_state->hw.crtc = from_plane_state->uapi.crtc;
>> + plane_state->hw.fb = from_plane_state->uapi.fb;
>> + if (plane_state->hw.fb)
>> + drm_framebuffer_get(plane_state->hw.fb);
>> +
>> + plane_state->hw.alpha = from_plane_state->uapi.alpha;
>> + plane_state->hw.pixel_blend_mode =
>> + from_plane_state->uapi.pixel_blend_mode;
>> + plane_state->hw.rotation = from_plane_state->uapi.rotation;
>> + plane_state->hw.color_encoding = from_plane_state->uapi.color_encoding;
>> + plane_state->hw.color_range = from_plane_state->uapi.color_range;
>> +}
>> +
>> int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_state,
>> struct intel_crtc_state *new_crtc_state,
>> const struct intel_plane_state *old_plane_state,
>> struct intel_plane_state *new_plane_state)
>> {
>> struct intel_plane *plane = to_intel_plane(new_plane_state->uapi.plane);
>> - const struct drm_framebuffer *fb = new_plane_state->hw.fb;
>> + const struct drm_framebuffer *fb;
>> int ret;
>>
>> + intel_plane_copy_uapi_to_hw_state(new_plane_state, new_plane_state);
>> + fb = new_plane_state->hw.fb;
>> +
>> new_crtc_state->active_planes &= ~BIT(plane->id);
>> new_crtc_state->nv12_planes &= ~BIT(plane->id);
>> new_crtc_state->c8_planes &= ~BIT(plane->id);
>> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
>> index e61e9a82aadf..cdb0f97d09f9 100644
>> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h
>> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
>> @@ -20,6 +20,8 @@ extern const struct drm_plane_helper_funcs intel_plane_helper_funcs;
>>
>> unsigned int intel_plane_data_rate(const struct intel_crtc_state *crtc_state,
>> const struct intel_plane_state *plane_state);
>> +void intel_plane_copy_uapi_to_hw_state(struct intel_plane_state *plane_state,
>> + const struct intel_plane_state *from_plane_state);
>> void intel_update_plane(struct intel_plane *plane,
>> const struct intel_crtc_state *crtc_state,
>> const struct intel_plane_state *plane_state);
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index 91a059e5fdcb..ed9504d0336e 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -3283,6 +3283,7 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
>>
>> plane_state->fb = fb;
>> plane_state->crtc = &intel_crtc->base;
>> + intel_plane_copy_uapi_to_hw_state(intel_state, intel_state);
>>
>> atomic_or(to_intel_plane(primary)->frontbuffer_bit,
>> &to_intel_frontbuffer(fb)->bits);
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
>> index e43d5a09550d..344b40687a30 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> @@ -523,11 +523,24 @@ struct intel_atomic_state {
>> };
>>
>> struct intel_plane_state {
>> - union {
>> - struct drm_plane_state base;
>> - struct drm_plane_state uapi;
>> - struct drm_plane_state hw;
>> - };
>> + struct drm_plane_state uapi;
>> +
>> + /*
>> + * actual hardware state, the state we program to the hardware.
>> + * The following members are used to verify the hardware state:
>> + * During initial hw readout, they need to be copied from uapi.
>> + */
>> + struct {
>> + struct drm_crtc *crtc;
>> + struct drm_framebuffer *fb;
>> +
>> + u16 alpha;
>> + uint16_t pixel_blend_mode;
> u16, though this will easily fit into u8.
>
>> + unsigned int rotation;
> This too can fit into u8.
>
> Would be nice to change those types in the core struct too I guess.
>
> Otherwise lgtm
> Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
I deliberately kept the drm core types, so that wouldn't cause problems. I think if we want to change it, we have to change it in core first. Not sure it's worth the time though. :)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
