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

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