[Intel-gfx] [PATCH 1/2] drm/i915/skl: Allow universal planes to position

[Matt Roper]

On Thu, Apr 02, 2015 at 10:08:27AM +0530, Jindal, Sonika wrote:
> 
> 
> On 4/1/2015 11:51 PM, Matt Roper wrote:
> >On Mon, Mar 30, 2015 at 02:04:56PM +0530, Sonika Jindal wrote:
> >>Signed-off-by: Sonika Jindal <sonika.jindal at intel.com>
> >
> >It looks like this is dependent on Ville's patch
> >
> >   [PATCH v2 6/9] drm/i915: Pass the primary plane position to .update_primary_plane()
> >
> >to actually let us do something sensible with the destination rectangle
> >at the hardware level.  Looks like that patch has a r-b, but hasn't made
> >it into di-nightly yet.
> >
> Right now, can_position is used to check for the scenarios where the
> primary plane is not covering the complete crtc. This could be due
> to positioning or a smaller fb on primary plane.
> With Ville's patch, we would be able to allow positioning to happen.
> But I need it here, to create a smaller fb for 90/270 rotation.
> 
> I agree that, until Ville's patch is there, we won't be entertaining
> any positioning requests on the primary plane and we will not be
> throwing any error also.

Right...and I think failing to throw an error would be seen as a bug,
which is why I think Ville's patch needs to go in first.  Since it's
already been reviewed, I'm not aware of anything holding that up from
happening.

> But for the 90/270 testcase in kms_rotation_crc to go through, we
> would need this to create a smaller fb so that we can rotate it.

So is your worry here that drm_plane_helper_check_update() doesn't
understand rotation and winds up mixing up width/height?  If so, I think
the proper course of action is to write a patch for the helper function
that makes it rotation-aware.


Matt

> 
> >Assuming Ville's patch lands first, this is
> >Reviewed-by: Matt Roper <matthew.d.roper at intel.com>
> >
> >
> >Matt
> >
> >>---
> >>  drivers/gpu/drm/i915/intel_display.c |    7 ++++++-
> >>  1 file changed, 6 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >>index ceb2e61..f0bbc22 100644
> >>--- a/drivers/gpu/drm/i915/intel_display.c
> >>+++ b/drivers/gpu/drm/i915/intel_display.c
> >>@@ -12150,16 +12150,21 @@ intel_check_primary_plane(struct drm_plane *plane,
> >>  	struct drm_rect *dest = &state->dst;
> >>  	struct drm_rect *src = &state->src;
> >>  	const struct drm_rect *clip = &state->clip;
> >>+	bool can_position = false;
> >>  	int ret;
> >>
> >>  	crtc = crtc ? crtc : plane->crtc;
> >>  	intel_crtc = to_intel_crtc(crtc);
> >>
> >>+	if (INTEL_INFO(dev)->gen >= 9)
> >>+		can_position = true;
> >>+
> >>  	ret = drm_plane_helper_check_update(plane, crtc, fb,
> >>  					    src, dest, clip,
> >>  					    DRM_PLANE_HELPER_NO_SCALING,
> >>  					    DRM_PLANE_HELPER_NO_SCALING,
> >>-					    false, true, &state->visible);
> >>+					    can_position, true,
> >>+					    &state->visible);
> >>  	if (ret)
> >>  		return ret;
> >>
> >>--
> >>1.7.10.4
> >>
> >>_______________________________________________
> >>Intel-gfx mailing list
> >>Intel-gfx at lists.freedesktop.org
> >>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795