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

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

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