Re: [PATCH 5/9] drm/i915: Setup clipped src/dest coordinates during FB reconstruction (v2)

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

Op 25-11-15 om 17:48 schreef Matt Roper:
> Plane state objects contain two copies of src/dest coordinates:  the
> original (requested by userspace) coordinates in the base
> drm_plane_state object, and a second, clipped copy (i.e., what we
> actually want to program to the hardware) in intel_plane_state.  We've
> only been setting up the former set of values during boot time FB
> reconstruction, but we should really be initializing both.
>
> Note that the code here probably still needs some more work since we
> make a lot of assumptions about how the BIOS programmed the hardware
> that may not always be true, especially on gen9+; e.g.,
>  * Primary plane might not be positioned at 0,0
>  * Primary plane could have been rotated by the BIOS
>  * Primary plane might be scaled
>  * The BIOS fb might be a single "extended mode" FB that spans
>    multiple displays.
>  * ...etc...
>
> v2: Reword/expand commit message description of assumptions we make
>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> Reviewed-by(v1): Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 73e9bf9..00e4c37 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2599,6 +2599,8 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
>  	struct drm_plane_state *plane_state = primary->state;
>  	struct drm_crtc_state *crtc_state = intel_crtc->base.state;
>  	struct intel_plane *intel_plane = to_intel_plane(primary);
> +	struct intel_plane_state *intel_state =
> +		to_intel_plane_state(plane_state);
>  	struct drm_framebuffer *fb;
>  
>  	if (!plane_config->fb)
> @@ -2659,6 +2661,15 @@ valid_fb:
>  	plane_state->crtc_w = fb->width;
>  	plane_state->crtc_h = fb->height;
>  
> +	intel_state->src.x1 = plane_state->src_x;
> +	intel_state->src.y1 = plane_state->src_y;
> +	intel_state->src.x2 = plane_state->src_x + plane_state->src_w;
> +	intel_state->src.y2 = plane_state->src_y + plane_state->src_h;
> +	intel_state->dst.x1 = plane_state->crtc_x;
> +	intel_state->dst.y1 = plane_state->crtc_y;
> +	intel_state->dst.x2 = plane_state->crtc_x + plane_state->crtc_w;
> +	intel_state->dst.y2 = plane_state->crtc_y + plane_state->crtc_h;
>
Why does it matter that those coordinates are set up? The first atomic commit would overwrite those anyway..
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx