On Thu, Dec 03, 2015 at 01:06:44PM +0100, Maarten Lankhorst wrote: > 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.ro...@intel.com> > > Reviewed-by(v1): Ville Syrjälä <ville.syrj...@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..
We potentially use these during watermark calculations when trying to calculate what we think sane watermark values would be for the hardware-readout state, which is before we get to our first commit. 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
