On Thu, Nov 12, 2015 at 04:31:59PM -0800, Matt Roper wrote: > 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. We seem to > be making two questionable assumptions: > * BIOS will program the primary plane to the entire display size > (potentially false on gen9+ platforms where the primary plane can be > windowed) We don't actually assume that on SKL+. We read out the plane size register and use that to set the initial fb dimensions. Although if the BIOS uses a scaled plane, we will get the wrong answer since then the plane size register will contain the dst size, not the src. > * The BIOS fb size actually matches the plane/display size. It seems > that there's nothing stopping the BIOS from overallocating the FB > (potentially to do an extended mode FB that spans multiple displays), > so setting our src/dest coordinates to the FB size here may not > always be right. I suppose we could have some kind of post-process step after the readout to see if all the planes actually scan out from different parts of the same fb. But that seems rather unlikely to me. The other assumption we do is that the BIOS sets up GTT to 1:1 map the stolen memory. Not sure if some BIOSen would for instance try to avoid the first page of stolen on gen8 (with EFI it should be doable I suppose, with VGA probably not), or maybe sets up a 90/270 rotated mapping instead. I have one BYT tablet where the BIOS uses 180 degree rotation, so even 90/270 degree rotation is not totally out of the question I think. > > Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx> > --- > 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 9fa041c..3145c9d 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2601,6 +2601,8 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, > struct drm_i915_gem_object *obj; > struct drm_plane *primary = intel_crtc->base.primary; > struct drm_plane_state *plane_state = primary->state; > + struct intel_plane_state *intel_state = > + to_intel_plane_state(plane_state); > struct drm_framebuffer *fb; > > if (!plane_config->fb) > @@ -2648,6 +2650,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; Patch looks sane. Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > + > obj = intel_fb_obj(fb); > if (obj->tiling_mode != I915_TILING_NONE) > dev_priv->preserve_bios_swizzle = true; > -- > 2.1.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
