Re: [PATCH 01/11] drm/i915: Use passed plane state for sprite planes.

Maarten Lankhorst
2015-10-22T13:11:36Z

Op 22-10-15 om 14:58 schreef Daniel Vetter:
> On Thu, Oct 22, 2015 at 01:56:26PM +0200, Maarten Lankhorst wrote:
>> Don't use plane->state directly, use the pointer from commit_plane.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_drv.h | 8 ++---
>> drivers/gpu/drm/i915/intel_sprite.c | 67 +++++++++++++++++++++++--------------
>> 2 files changed, 45 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 4e35557bbd41..51722e657b91 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -631,16 +631,16 @@ struct intel_plane {
>> /*
>> * NOTE: Do not place new plane state fields here (e.g., when adding
>> * new plane properties). New runtime state should now be placed in
>> - * the intel_plane_state structure and accessed via drm_plane->state.
>> + * the intel_plane_state structure and accessed via plane_state.
>> */
>>
>> void (*update_plane)(struct drm_plane *plane,
>> - struct drm_crtc *crtc,
>> - struct drm_framebuffer *fb,
>> + struct intel_plane_state *plane_state,
>> int crtc_x, int crtc_y,
>> unsigned int crtc_w, unsigned int crtc_h,
>> uint32_t x, uint32_t y,
>> - uint32_t src_w, uint32_t src_h);
>> + uint32_t src_w, uint32_t src_h,
>> + struct intel_scaler *scaler);
> This is an ugly hack imo. Better to add the scaler pointer to the plane
> state, set it in compute_config once we're done and not pass it around
> like this. Or we also pass around a pointer to the right crtc_state.
This won't be valid if the crtc_state is updated after the page flip, so I don't think it's a
good idea to add it to the state. But we could pass the crtc_state here and nuke the rest.

> Also, if we pass around plane_state I think all the crtc_x/y/w/h and x/y
> and src_x/y/w/h should disappear, for a really clean vfunc interface.
Agreed.

~Maarten