Re: [PATCH 5/7] drm/i915: Prepare for atomic plane helpers

[Matt Roper <matthew.d.roper@intel.com>]

On Thu, Nov 13, 2014 at 11:46:25AM -0800, Bob Paauwe wrote:
> On Thu, 13 Nov 2014 10:43:24 -0800
> Matt Roper <matthew.d.roper@intel.com> wrote:
...
> > +
> > +/**
> > + * intel_plane_destroy_state - destroy plane state
> > + * @plane: drm plane
> > + *
> > + * Allocates and returns a copy of the plane state (both common and
> > + * Intel-specific) for the specified plane.
> 
> Comment should be updated for destroy.

Good catch; will update.

...
> > +static int intel_plane_atomic_check(struct drm_plane *plane,
> > +				    struct drm_plane_state *state)
> > +{
> > +	struct intel_plane *intel_plane = to_intel_plane(plane);
> > +	struct intel_crtc *intel_crtc = to_intel_crtc(state->crtc);
> > +	struct intel_plane_state *intel_state = to_intel_plane_state(state);
> > +
> > +	/* Disabling a plane is always okay */
> > +	if (state->fb == NULL)
> > +		return 0;
> > +
> > +	/*
> > +	 * The original src/dest coordinates are stored in state->base, but
> > +	 * we want to keep another copy internal to our driver that we can
> > +	 * clip/modify ourselves.
> > +	 */
> > +	intel_state->src.x1 = state->src_x;
> > +	intel_state->src.y1 = state->src_y;
> > +	intel_state->src.x2 = state->src_x + state->src_w;
> > +	intel_state->src.y2 = state->src_y + state->src_h;
> > +	intel_state->dst.x1 = state->crtc_x;
> > +	intel_state->dst.y1 = state->crtc_y;
> > +	intel_state->dst.x2 = state->crtc_x + state->crtc_w;
> > +	intel_state->dst.y2 = state->crtc_y + state->crtc_h;
> > +
> > +	/* Clip all planes to CRTC size, or 0x0 if CRTC is disabled */
> > +	if (intel_crtc) {
> > +		intel_state->clip.x1 = 0;
> > +		intel_state->clip.y1 = 0;
> > +		intel_state->clip.x2 =
> > +			intel_crtc->active ? intel_crtc->config.pipe_src_w : 0;
> > +		intel_state->clip.y2 =
> > +			intel_crtc->active ? intel_crtc->config.pipe_src_h : 0;
> 
> Should this be using intel_crtc->new_config here?  I'm not entierly
> clear on how the crtc config/new_config would tie into the atomic state.

I don't think so.  config vs new_config tracks pipe configuration during
the modeset sequence, but we're only doing a plane update here, so the
current crtc configuration is what we'd clip against.  I haven't
converted over any of the CRTC modeset handling, just the plane updates.


Matt

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx