| Message ID | 1447847366-2431-1-git-send-email-nabendu.bikash.maiti@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Nov 18, 2015 at 05:19:26PM +0530, Nabendu Maiti wrote: > On 90/270 rotation case source width and height was not compared > properly with destination height and width check plane.Which added > erroneous check while doing scaling or normal 90/270 rotation. > > Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 22 ++++++++++++++++++++-- > 1 file changed, 20 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 688d484..cd5bb28 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13723,6 +13723,9 @@ intel_check_primary_plane(struct drm_plane *plane, > int min_scale = DRM_PLANE_HELPER_NO_SCALING; > int max_scale = DRM_PLANE_HELPER_NO_SCALING; > bool can_position = false; > + struct drm_rect *src = &state->src; > + struct drm_rect *dest = &state->dst; > + int ret = -1; > > /* use scaler when colorkey is not required */ > if (INTEL_INFO(plane->dev)->gen >= 9 && > @@ -13732,11 +13735,26 @@ intel_check_primary_plane(struct drm_plane *plane, > can_position = true; > } > > - return drm_plane_helper_check_update(plane, crtc, fb, &state->src, > - &state->dst, &state->clip, > + /* > + * FIXME the following code does a bunch of fuzzy adjustments to the > + * coordinates and sizes for rotations. We probably need some way to > + * decide whether more strict checking should be done instead. > + */ > + if (fb) > + drm_rect_rotate(src, fb->width << 16, fb->height << 16, > + state->base.rotation); > + > + ret = drm_plane_helper_check_update(plane, crtc, fb, src, > + dest, &state->clip, > min_scale, max_scale, > can_position, true, > &state->visible); > + > + /* Restore the originl unrotated co-ordinates */ > + if (fb) > + drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, > + state->base.rotation); We should put the rotation handling into helper. And someone should really just move all the good code from intel_sprite into the helper instead of having two totally different ways of doing things. > + return ret; > } > > static void > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 11/18/2015 05:44 PM, Ville Syrjälä wrote: > On Wed, Nov 18, 2015 at 05:19:26PM +0530, Nabendu Maiti wrote: >> On 90/270 rotation case source width and height was not compared >> properly with destination height and width check plane.Which added >> erroneous check while doing scaling or normal 90/270 rotation. >> >> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com> >> --- >> drivers/gpu/drm/i915/intel_display.c | 22 ++++++++++++++++++++-- >> 1 file changed, 20 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index 688d484..cd5bb28 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -13723,6 +13723,9 @@ intel_check_primary_plane(struct drm_plane *plane, >> int min_scale = DRM_PLANE_HELPER_NO_SCALING; >> int max_scale = DRM_PLANE_HELPER_NO_SCALING; >> bool can_position = false; >> + struct drm_rect *src = &state->src; >> + struct drm_rect *dest = &state->dst; >> + int ret = -1; >> >> /* use scaler when colorkey is not required */ >> if (INTEL_INFO(plane->dev)->gen >= 9 && >> @@ -13732,11 +13735,26 @@ intel_check_primary_plane(struct drm_plane *plane, >> can_position = true; >> } >> >> - return drm_plane_helper_check_update(plane, crtc, fb, &state->src, >> - &state->dst, &state->clip, >> + /* >> + * FIXME the following code does a bunch of fuzzy adjustments to the >> + * coordinates and sizes for rotations. We probably need some way to >> + * decide whether more strict checking should be done instead. >> + */ >> + if (fb) >> + drm_rect_rotate(src, fb->width << 16, fb->height << 16, >> + state->base.rotation); >> + >> + ret = drm_plane_helper_check_update(plane, crtc, fb, src, >> + dest, &state->clip, >> min_scale, max_scale, >> can_position, true, >> &state->visible); >> + >> + /* Restore the originl unrotated co-ordinates */ >> + if (fb) >> + drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, >> + state->base.rotation); > > We should put the rotation handling into helper. And someone should > really just move all the good code from intel_sprite into the helper > instead of having two totally different ways of doing things. Yes right.I learned that there is an effort going on unification of primary and sprite plane. To put this in helper function is a bigger modifications . In the meantime in android we need this fix. So if you suggest I will hold the patch until the unification completes anf float it once again if required. > >> + return ret; >> } >> >> static void >> -- >> 1.9.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 11/27/2015 12:10 AM, Nabendu Maiti wrote: > > > On 11/18/2015 05:44 PM, Ville Syrjälä wrote: >> On Wed, Nov 18, 2015 at 05:19:26PM +0530, Nabendu Maiti wrote: >>> On 90/270 rotation case source width and height was not compared >>> properly with destination height and width check plane.Which added >>> erroneous check while doing scaling or normal 90/270 rotation. >>> >>> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com> >>> --- >>> drivers/gpu/drm/i915/intel_display.c | 22 ++++++++++++++++++++-- >>> 1 file changed, 20 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_display.c >>> b/drivers/gpu/drm/i915/intel_display.c >>> index 688d484..cd5bb28 100644 >>> --- a/drivers/gpu/drm/i915/intel_display.c >>> +++ b/drivers/gpu/drm/i915/intel_display.c >>> @@ -13723,6 +13723,9 @@ intel_check_primary_plane(struct drm_plane >>> *plane, >>> int min_scale = DRM_PLANE_HELPER_NO_SCALING; >>> int max_scale = DRM_PLANE_HELPER_NO_SCALING; >>> bool can_position = false; >>> + struct drm_rect *src = &state->src; >>> + struct drm_rect *dest = &state->dst; >>> + int ret = -1; >>> /* use scaler when colorkey is not required */ >>> if (INTEL_INFO(plane->dev)->gen >= 9 && >>> @@ -13732,11 +13735,26 @@ intel_check_primary_plane(struct drm_plane >>> *plane, >>> can_position = true; >>> } >>> - return drm_plane_helper_check_update(plane, crtc, fb, >>> &state->src, >>> - &state->dst, &state->clip, >>> + /* >>> + * FIXME the following code does a bunch of fuzzy adjustments >>> to the >>> + * coordinates and sizes for rotations. We probably need some >>> way to >>> + * decide whether more strict checking should be done instead. >>> + */ >>> + if (fb) >>> + drm_rect_rotate(src, fb->width << 16, fb->height << 16, >>> + state->base.rotation); >>> + >>> + ret = drm_plane_helper_check_update(plane, crtc, fb, src, >>> + dest, &state->clip, >>> min_scale, max_scale, >>> can_position, true, >>> &state->visible); >>> + >>> + /* Restore the originl unrotated co-ordinates */ >>> + if (fb) >>> + drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, >>> + state->base.rotation); >> >> We should put the rotation handling into helper. And someone should >> really just move all the good code from intel_sprite into the helper >> instead of having two totally different ways of doing things. > Yes right.I learned that there is an effort going on unification of > primary and sprite plane. To put this in helper function is a bigger > modifications . > In the meantime in android we need this fix. So if you suggest I will > hold the patch until the unification completes anf float it once again > if required. Any update on this patch or unification patch? >> >>> + return ret; >>> } >>> static void >>> -- >>> 1.9.1 >>> >>> _______________________________________________ >>> Intel-gfx mailing list >>> Intel-gfx@lists.freedesktop.org >>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 688d484..cd5bb28 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13723,6 +13723,9 @@ intel_check_primary_plane(struct drm_plane *plane, int min_scale = DRM_PLANE_HELPER_NO_SCALING; int max_scale = DRM_PLANE_HELPER_NO_SCALING; bool can_position = false; + struct drm_rect *src = &state->src; + struct drm_rect *dest = &state->dst; + int ret = -1; /* use scaler when colorkey is not required */ if (INTEL_INFO(plane->dev)->gen >= 9 && @@ -13732,11 +13735,26 @@ intel_check_primary_plane(struct drm_plane *plane, can_position = true; } - return drm_plane_helper_check_update(plane, crtc, fb, &state->src, - &state->dst, &state->clip, + /* + * FIXME the following code does a bunch of fuzzy adjustments to the + * coordinates and sizes for rotations. We probably need some way to + * decide whether more strict checking should be done instead. + */ + if (fb) + drm_rect_rotate(src, fb->width << 16, fb->height << 16, + state->base.rotation); + + ret = drm_plane_helper_check_update(plane, crtc, fb, src, + dest, &state->clip, min_scale, max_scale, can_position, true, &state->visible); + + /* Restore the originl unrotated co-ordinates */ + if (fb) + drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, + state->base.rotation); + return ret; } static void
On 90/270 rotation case source width and height was not compared properly with destination height and width check plane.Which added erroneous check while doing scaling or normal 90/270 rotation. Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-)