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

Matt Roper
2014-11-13T21:31:38Z

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