| Message ID | 1408030398-18547-1-git-send-email-thomas.wood@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Aug 14, 2014 at 04:33:18PM +0100, Thomas Wood wrote: > Make sure plane rotation is reset correctly when restoring the fbdev > configuration by using drm_mode_plane_set_obj_prop. This calls the > driver's set_property callback and ensures the rotation is actually > changed. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82236 > Signed-off-by: Thomas Wood <thomas.wood@intel.com> Commit message is missing the citation of the offending commit that introduced this. With that addressed this is Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> And please cc all the people involved in the offending commit next time around, too. -Daniel > --- > drivers/gpu/drm/drm_crtc.c | 25 ++++++++++++++++++++----- > drivers/gpu/drm/drm_fb_helper.c | 6 +++--- > include/drm/drm_crtc.h | 3 +++ > 3 files changed, 26 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index f09b752..95f330a 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -4175,12 +4175,25 @@ static int drm_mode_crtc_set_obj_prop(struct drm_mode_object *obj, > return ret; > } > > -static int drm_mode_plane_set_obj_prop(struct drm_mode_object *obj, > - struct drm_property *property, > - uint64_t value) > +/** > + * drm_mode_plane_set_obj_prop - set the value of a property > + * @plane: drm plane object to set property value for > + * @property: property to set > + * @val: value the property should be set to > + * > + * This functions sets a given property on a given plane object. This function > + * calls the driver's ->set_property callback and changes the software state of > + * the property if the callback succeeds. > + * > + * Returns: > + * Zero on success, error code on failure. > + */ > +int drm_mode_plane_set_obj_prop(struct drm_plane *plane, > + struct drm_property *property, > + uint64_t value) > { > int ret = -EINVAL; > - struct drm_plane *plane = obj_to_plane(obj); > + struct drm_mode_object *obj = &plane->base; > > if (plane->funcs->set_property) > ret = plane->funcs->set_property(plane, property, value); > @@ -4189,6 +4202,7 @@ static int drm_mode_plane_set_obj_prop(struct drm_mode_object *obj, > > return ret; > } > +EXPORT_SYMBOL(drm_mode_plane_set_obj_prop); > > /** > * drm_mode_getproperty_ioctl - get the current value of a object's property > @@ -4327,7 +4341,8 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data, > ret = drm_mode_crtc_set_obj_prop(arg_obj, property, arg->value); > break; > case DRM_MODE_OBJECT_PLANE: > - ret = drm_mode_plane_set_obj_prop(arg_obj, property, arg->value); > + ret = drm_mode_plane_set_obj_prop(obj_to_plane(arg_obj), > + property, arg->value); > break; > } > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index 63d7b8e..0c0c39b 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -296,9 +296,9 @@ static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper) > drm_plane_force_disable(plane); > > if (dev->mode_config.rotation_property) { > - drm_object_property_set_value(&plane->base, > - dev->mode_config.rotation_property, > - BIT(DRM_ROTATE_0)); > + drm_mode_plane_set_obj_prop(plane, > + dev->mode_config.rotation_property, > + BIT(DRM_ROTATE_0)); > } > } > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index 0375d75..31344bf 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -1127,6 +1127,9 @@ extern int drm_mode_obj_get_properties_ioctl(struct drm_device *dev, void *data, > struct drm_file *file_priv); > extern int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data, > struct drm_file *file_priv); > +extern int drm_mode_plane_set_obj_prop(struct drm_plane *plane, > + struct drm_property *property, > + uint64_t value); > > extern void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth, > int *bpp); > -- > 1.9.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 14 August 2014 17:02, Daniel Vetter <daniel@ffwll.ch> wrote: > On Thu, Aug 14, 2014 at 04:33:18PM +0100, Thomas Wood wrote: >> Make sure plane rotation is reset correctly when restoring the fbdev >> configuration by using drm_mode_plane_set_obj_prop. This calls the >> driver's set_property callback and ensures the rotation is actually >> changed. >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82236 >> Signed-off-by: Thomas Wood <thomas.wood@intel.com> > > Commit message is missing the citation of the offending commit that > introduced this. With that addressed this is > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > And please cc all the people involved in the offending commit next time > around, too. The "reset" feature was introduced in commit 9783de2 (drm: Resetting rotation property), although it also looks like the issue was originally addressed in a previous version of the patch: http://lists.freedesktop.org/archives/dri-devel/2014-July/062981.html Although this version calls the driver's set_property directly rather than exporting drm_mode_plane_set_obj_prop. The final version of the patch contains a different change: > -Daniel >> --- >> drivers/gpu/drm/drm_crtc.c | 25 ++++++++++++++++++++----- >> drivers/gpu/drm/drm_fb_helper.c | 6 +++--- >> include/drm/drm_crtc.h | 3 +++ >> 3 files changed, 26 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c >> index f09b752..95f330a 100644 >> --- a/drivers/gpu/drm/drm_crtc.c >> +++ b/drivers/gpu/drm/drm_crtc.c >> @@ -4175,12 +4175,25 @@ static int drm_mode_crtc_set_obj_prop(struct drm_mode_object *obj, >> return ret; >> } >> >> -static int drm_mode_plane_set_obj_prop(struct drm_mode_object *obj, >> - struct drm_property *property, >> - uint64_t value) >> +/** >> + * drm_mode_plane_set_obj_prop - set the value of a property >> + * @plane: drm plane object to set property value for >> + * @property: property to set >> + * @val: value the property should be set to >> + * >> + * This functions sets a given property on a given plane object. This function >> + * calls the driver's ->set_property callback and changes the software state of >> + * the property if the callback succeeds. >> + * >> + * Returns: >> + * Zero on success, error code on failure. >> + */ >> +int drm_mode_plane_set_obj_prop(struct drm_plane *plane, >> + struct drm_property *property, >> + uint64_t value) >> { >> int ret = -EINVAL; >> - struct drm_plane *plane = obj_to_plane(obj); >> + struct drm_mode_object *obj = &plane->base; >> >> if (plane->funcs->set_property) >> ret = plane->funcs->set_property(plane, property, value); >> @@ -4189,6 +4202,7 @@ static int drm_mode_plane_set_obj_prop(struct drm_mode_object *obj, >> >> return ret; >> } >> +EXPORT_SYMBOL(drm_mode_plane_set_obj_prop); >> >> /** >> * drm_mode_getproperty_ioctl - get the current value of a object's property >> @@ -4327,7 +4341,8 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data, >> ret = drm_mode_crtc_set_obj_prop(arg_obj, property, arg->value); >> break; >> case DRM_MODE_OBJECT_PLANE: >> - ret = drm_mode_plane_set_obj_prop(arg_obj, property, arg->value); >> + ret = drm_mode_plane_set_obj_prop(obj_to_plane(arg_obj), >> + property, arg->value); >> break; >> } >> >> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c >> index 63d7b8e..0c0c39b 100644 >> --- a/drivers/gpu/drm/drm_fb_helper.c >> +++ b/drivers/gpu/drm/drm_fb_helper.c >> @@ -296,9 +296,9 @@ static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper) >> drm_plane_force_disable(plane); >> >> if (dev->mode_config.rotation_property) { >> - drm_object_property_set_value(&plane->base, >> - dev->mode_config.rotation_property, >> - BIT(DRM_ROTATE_0)); >> + drm_mode_plane_set_obj_prop(plane, >> + dev->mode_config.rotation_property, >> + BIT(DRM_ROTATE_0)); >> } >> } >> >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h >> index 0375d75..31344bf 100644 >> --- a/include/drm/drm_crtc.h >> +++ b/include/drm/drm_crtc.h >> @@ -1127,6 +1127,9 @@ extern int drm_mode_obj_get_properties_ioctl(struct drm_device *dev, void *data, >> struct drm_file *file_priv); >> extern int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data, >> struct drm_file *file_priv); >> +extern int drm_mode_plane_set_obj_prop(struct drm_plane *plane, >> + struct drm_property *property, >> + uint64_t value); >> >> extern void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth, >> int *bpp); >> -- >> 1.9.3 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 15 August 2014 10:42, Thomas Wood <thomas.wood@intel.com> wrote: > On 14 August 2014 17:02, Daniel Vetter <daniel@ffwll.ch> wrote: >> On Thu, Aug 14, 2014 at 04:33:18PM +0100, Thomas Wood wrote: >>> Make sure plane rotation is reset correctly when restoring the fbdev >>> configuration by using drm_mode_plane_set_obj_prop. This calls the >>> driver's set_property callback and ensures the rotation is actually >>> changed. >>> >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82236 >>> Signed-off-by: Thomas Wood <thomas.wood@intel.com> >> >> Commit message is missing the citation of the offending commit that >> introduced this. With that addressed this is >> >> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> >> >> And please cc all the people involved in the offending commit next time >> around, too. > > The "reset" feature was introduced in commit 9783de2 (drm: Resetting > rotation property), although it also looks like the issue was > originally addressed in a previous version of the patch: > > http://lists.freedesktop.org/archives/dri-devel/2014-July/062981.html > > Although this version calls the driver's set_property directly rather > than exporting drm_mode_plane_set_obj_prop. > > The final version of the patch contains a different change: http://lists.freedesktop.org/archives/dri-devel/2014-August/065728.html So there were actually two different "v2" versions of the patch. > >> -Daniel >>> --- >>> drivers/gpu/drm/drm_crtc.c | 25 ++++++++++++++++++++----- >>> drivers/gpu/drm/drm_fb_helper.c | 6 +++--- >>> include/drm/drm_crtc.h | 3 +++ >>> 3 files changed, 26 insertions(+), 8 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c >>> index f09b752..95f330a 100644 >>> --- a/drivers/gpu/drm/drm_crtc.c >>> +++ b/drivers/gpu/drm/drm_crtc.c >>> @@ -4175,12 +4175,25 @@ static int drm_mode_crtc_set_obj_prop(struct drm_mode_object *obj, >>> return ret; >>> } >>> >>> -static int drm_mode_plane_set_obj_prop(struct drm_mode_object *obj, >>> - struct drm_property *property, >>> - uint64_t value) >>> +/** >>> + * drm_mode_plane_set_obj_prop - set the value of a property >>> + * @plane: drm plane object to set property value for >>> + * @property: property to set >>> + * @val: value the property should be set to >>> + * >>> + * This functions sets a given property on a given plane object. This function >>> + * calls the driver's ->set_property callback and changes the software state of >>> + * the property if the callback succeeds. >>> + * >>> + * Returns: >>> + * Zero on success, error code on failure. >>> + */ >>> +int drm_mode_plane_set_obj_prop(struct drm_plane *plane, >>> + struct drm_property *property, >>> + uint64_t value) >>> { >>> int ret = -EINVAL; >>> - struct drm_plane *plane = obj_to_plane(obj); >>> + struct drm_mode_object *obj = &plane->base; >>> >>> if (plane->funcs->set_property) >>> ret = plane->funcs->set_property(plane, property, value); >>> @@ -4189,6 +4202,7 @@ static int drm_mode_plane_set_obj_prop(struct drm_mode_object *obj, >>> >>> return ret; >>> } >>> +EXPORT_SYMBOL(drm_mode_plane_set_obj_prop); >>> >>> /** >>> * drm_mode_getproperty_ioctl - get the current value of a object's property >>> @@ -4327,7 +4341,8 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data, >>> ret = drm_mode_crtc_set_obj_prop(arg_obj, property, arg->value); >>> break; >>> case DRM_MODE_OBJECT_PLANE: >>> - ret = drm_mode_plane_set_obj_prop(arg_obj, property, arg->value); >>> + ret = drm_mode_plane_set_obj_prop(obj_to_plane(arg_obj), >>> + property, arg->value); >>> break; >>> } >>> >>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c >>> index 63d7b8e..0c0c39b 100644 >>> --- a/drivers/gpu/drm/drm_fb_helper.c >>> +++ b/drivers/gpu/drm/drm_fb_helper.c >>> @@ -296,9 +296,9 @@ static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper) >>> drm_plane_force_disable(plane); >>> >>> if (dev->mode_config.rotation_property) { >>> - drm_object_property_set_value(&plane->base, >>> - dev->mode_config.rotation_property, >>> - BIT(DRM_ROTATE_0)); >>> + drm_mode_plane_set_obj_prop(plane, >>> + dev->mode_config.rotation_property, >>> + BIT(DRM_ROTATE_0)); >>> } >>> } >>> >>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h >>> index 0375d75..31344bf 100644 >>> --- a/include/drm/drm_crtc.h >>> +++ b/include/drm/drm_crtc.h >>> @@ -1127,6 +1127,9 @@ extern int drm_mode_obj_get_properties_ioctl(struct drm_device *dev, void *data, >>> struct drm_file *file_priv); >>> extern int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data, >>> struct drm_file *file_priv); >>> +extern int drm_mode_plane_set_obj_prop(struct drm_plane *plane, >>> + struct drm_property *property, >>> + uint64_t value); >>> >>> extern void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth, >>> int *bpp); >>> -- >>> 1.9.3 >>> >>> _______________________________________________ >>> Intel-gfx mailing list >>> Intel-gfx@lists.freedesktop.org >>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >> >> -- >> Daniel Vetter >> Software Engineer, Intel Corporation >> +41 (0) 79 365 57 48 - http://blog.ffwll.ch >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Aug 15, 2014 at 10:48:05AM +0100, Thomas Wood wrote: > On 15 August 2014 10:42, Thomas Wood <thomas.wood@intel.com> wrote: > > On 14 August 2014 17:02, Daniel Vetter <daniel@ffwll.ch> wrote: > >> On Thu, Aug 14, 2014 at 04:33:18PM +0100, Thomas Wood wrote: > >>> Make sure plane rotation is reset correctly when restoring the fbdev > >>> configuration by using drm_mode_plane_set_obj_prop. This calls the > >>> driver's set_property callback and ensures the rotation is actually > >>> changed. > >>> > >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82236 > >>> Signed-off-by: Thomas Wood <thomas.wood@intel.com> > >> > >> Commit message is missing the citation of the offending commit that > >> introduced this. With that addressed this is > >> > >> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > >> > >> And please cc all the people involved in the offending commit next time > >> around, too. > > > > The "reset" feature was introduced in commit 9783de2 (drm: Resetting > > rotation property), although it also looks like the issue was > > originally addressed in a previous version of the patch: > > > > http://lists.freedesktop.org/archives/dri-devel/2014-July/062981.html > > > > Although this version calls the driver's set_property directly rather > > than exporting drm_mode_plane_set_obj_prop. > > > > The final version of the patch contains a different change: > > http://lists.freedesktop.org/archives/dri-devel/2014-August/065728.html > > So there were actually two different "v2" versions of the patch. Somehow I've thought this is for -fixes, but the patch is only in dinq. So merged, thanks. -Daniel > > > > > > >> -Daniel > >>> --- > >>> drivers/gpu/drm/drm_crtc.c | 25 ++++++++++++++++++++----- > >>> drivers/gpu/drm/drm_fb_helper.c | 6 +++--- > >>> include/drm/drm_crtc.h | 3 +++ > >>> 3 files changed, 26 insertions(+), 8 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > >>> index f09b752..95f330a 100644 > >>> --- a/drivers/gpu/drm/drm_crtc.c > >>> +++ b/drivers/gpu/drm/drm_crtc.c > >>> @@ -4175,12 +4175,25 @@ static int drm_mode_crtc_set_obj_prop(struct drm_mode_object *obj, > >>> return ret; > >>> } > >>> > >>> -static int drm_mode_plane_set_obj_prop(struct drm_mode_object *obj, > >>> - struct drm_property *property, > >>> - uint64_t value) > >>> +/** > >>> + * drm_mode_plane_set_obj_prop - set the value of a property > >>> + * @plane: drm plane object to set property value for > >>> + * @property: property to set > >>> + * @val: value the property should be set to > >>> + * > >>> + * This functions sets a given property on a given plane object. This function > >>> + * calls the driver's ->set_property callback and changes the software state of > >>> + * the property if the callback succeeds. > >>> + * > >>> + * Returns: > >>> + * Zero on success, error code on failure. > >>> + */ > >>> +int drm_mode_plane_set_obj_prop(struct drm_plane *plane, > >>> + struct drm_property *property, > >>> + uint64_t value) > >>> { > >>> int ret = -EINVAL; > >>> - struct drm_plane *plane = obj_to_plane(obj); > >>> + struct drm_mode_object *obj = &plane->base; > >>> > >>> if (plane->funcs->set_property) > >>> ret = plane->funcs->set_property(plane, property, value); > >>> @@ -4189,6 +4202,7 @@ static int drm_mode_plane_set_obj_prop(struct drm_mode_object *obj, > >>> > >>> return ret; > >>> } > >>> +EXPORT_SYMBOL(drm_mode_plane_set_obj_prop); > >>> > >>> /** > >>> * drm_mode_getproperty_ioctl - get the current value of a object's property > >>> @@ -4327,7 +4341,8 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data, > >>> ret = drm_mode_crtc_set_obj_prop(arg_obj, property, arg->value); > >>> break; > >>> case DRM_MODE_OBJECT_PLANE: > >>> - ret = drm_mode_plane_set_obj_prop(arg_obj, property, arg->value); > >>> + ret = drm_mode_plane_set_obj_prop(obj_to_plane(arg_obj), > >>> + property, arg->value); > >>> break; > >>> } > >>> > >>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > >>> index 63d7b8e..0c0c39b 100644 > >>> --- a/drivers/gpu/drm/drm_fb_helper.c > >>> +++ b/drivers/gpu/drm/drm_fb_helper.c > >>> @@ -296,9 +296,9 @@ static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper) > >>> drm_plane_force_disable(plane); > >>> > >>> if (dev->mode_config.rotation_property) { > >>> - drm_object_property_set_value(&plane->base, > >>> - dev->mode_config.rotation_property, > >>> - BIT(DRM_ROTATE_0)); > >>> + drm_mode_plane_set_obj_prop(plane, > >>> + dev->mode_config.rotation_property, > >>> + BIT(DRM_ROTATE_0)); > >>> } > >>> } > >>> > >>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > >>> index 0375d75..31344bf 100644 > >>> --- a/include/drm/drm_crtc.h > >>> +++ b/include/drm/drm_crtc.h > >>> @@ -1127,6 +1127,9 @@ extern int drm_mode_obj_get_properties_ioctl(struct drm_device *dev, void *data, > >>> struct drm_file *file_priv); > >>> extern int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data, > >>> struct drm_file *file_priv); > >>> +extern int drm_mode_plane_set_obj_prop(struct drm_plane *plane, > >>> + struct drm_property *property, > >>> + uint64_t value); > >>> > >>> extern void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth, > >>> int *bpp); > >>> -- > >>> 1.9.3 > >>> > >>> _______________________________________________ > >>> Intel-gfx mailing list > >>> Intel-gfx@lists.freedesktop.org > >>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > >> > >> -- > >> Daniel Vetter > >> Software Engineer, Intel Corporation > >> +41 (0) 79 365 57 48 - http://blog.ffwll.ch > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 15 August 2014 11:22, Daniel Vetter <daniel@ffwll.ch> wrote: > On Fri, Aug 15, 2014 at 10:48:05AM +0100, Thomas Wood wrote: >> On 15 August 2014 10:42, Thomas Wood <thomas.wood@intel.com> wrote: >> > On 14 August 2014 17:02, Daniel Vetter <daniel@ffwll.ch> wrote: >> >> On Thu, Aug 14, 2014 at 04:33:18PM +0100, Thomas Wood wrote: >> >>> Make sure plane rotation is reset correctly when restoring the fbdev >> >>> configuration by using drm_mode_plane_set_obj_prop. This calls the >> >>> driver's set_property callback and ensures the rotation is actually >> >>> changed. >> >>> >> >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82236 >> >>> Signed-off-by: Thomas Wood <thomas.wood@intel.com> >> >> >> >> Commit message is missing the citation of the offending commit that >> >> introduced this. With that addressed this is >> >> >> >> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> >> >> >> >> And please cc all the people involved in the offending commit next time >> >> around, too. >> > >> > The "reset" feature was introduced in commit 9783de2 (drm: Resetting >> > rotation property), although it also looks like the issue was >> > originally addressed in a previous version of the patch: >> > >> > http://lists.freedesktop.org/archives/dri-devel/2014-July/062981.html >> > >> > Although this version calls the driver's set_property directly rather >> > than exporting drm_mode_plane_set_obj_prop. >> > >> > The final version of the patch contains a different change: >> >> http://lists.freedesktop.org/archives/dri-devel/2014-August/065728.html >> >> So there were actually two different "v2" versions of the patch. > > Somehow I've thought this is for -fixes, but the patch is only in dinq. So > merged, thanks. > -Daniel Is there any preference to calling set_property directly (as originally fixed), as opposed to exporting and using drm_mode_plane_set_obj_prop? I have also pushed a patch for igt/kms_rotate_crc to add a testcase for this issue: http://lists.freedesktop.org/archives/intel-gfx/2014-August/050775.html > >> >> >> >> > >> >> -Daniel >> >>> --- >> >>> drivers/gpu/drm/drm_crtc.c | 25 ++++++++++++++++++++----- >> >>> drivers/gpu/drm/drm_fb_helper.c | 6 +++--- >> >>> include/drm/drm_crtc.h | 3 +++ >> >>> 3 files changed, 26 insertions(+), 8 deletions(-) >> >>> >> >>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c >> >>> index f09b752..95f330a 100644 >> >>> --- a/drivers/gpu/drm/drm_crtc.c >> >>> +++ b/drivers/gpu/drm/drm_crtc.c >> >>> @@ -4175,12 +4175,25 @@ static int drm_mode_crtc_set_obj_prop(struct drm_mode_object *obj, >> >>> return ret; >> >>> } >> >>> >> >>> -static int drm_mode_plane_set_obj_prop(struct drm_mode_object *obj, >> >>> - struct drm_property *property, >> >>> - uint64_t value) >> >>> +/** >> >>> + * drm_mode_plane_set_obj_prop - set the value of a property >> >>> + * @plane: drm plane object to set property value for >> >>> + * @property: property to set >> >>> + * @val: value the property should be set to >> >>> + * >> >>> + * This functions sets a given property on a given plane object. This function >> >>> + * calls the driver's ->set_property callback and changes the software state of >> >>> + * the property if the callback succeeds. >> >>> + * >> >>> + * Returns: >> >>> + * Zero on success, error code on failure. >> >>> + */ >> >>> +int drm_mode_plane_set_obj_prop(struct drm_plane *plane, >> >>> + struct drm_property *property, >> >>> + uint64_t value) >> >>> { >> >>> int ret = -EINVAL; >> >>> - struct drm_plane *plane = obj_to_plane(obj); >> >>> + struct drm_mode_object *obj = &plane->base; >> >>> >> >>> if (plane->funcs->set_property) >> >>> ret = plane->funcs->set_property(plane, property, value); >> >>> @@ -4189,6 +4202,7 @@ static int drm_mode_plane_set_obj_prop(struct drm_mode_object *obj, >> >>> >> >>> return ret; >> >>> } >> >>> +EXPORT_SYMBOL(drm_mode_plane_set_obj_prop); >> >>> >> >>> /** >> >>> * drm_mode_getproperty_ioctl - get the current value of a object's property >> >>> @@ -4327,7 +4341,8 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data, >> >>> ret = drm_mode_crtc_set_obj_prop(arg_obj, property, arg->value); >> >>> break; >> >>> case DRM_MODE_OBJECT_PLANE: >> >>> - ret = drm_mode_plane_set_obj_prop(arg_obj, property, arg->value); >> >>> + ret = drm_mode_plane_set_obj_prop(obj_to_plane(arg_obj), >> >>> + property, arg->value); >> >>> break; >> >>> } >> >>> >> >>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c >> >>> index 63d7b8e..0c0c39b 100644 >> >>> --- a/drivers/gpu/drm/drm_fb_helper.c >> >>> +++ b/drivers/gpu/drm/drm_fb_helper.c >> >>> @@ -296,9 +296,9 @@ static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper) >> >>> drm_plane_force_disable(plane); >> >>> >> >>> if (dev->mode_config.rotation_property) { >> >>> - drm_object_property_set_value(&plane->base, >> >>> - dev->mode_config.rotation_property, >> >>> - BIT(DRM_ROTATE_0)); >> >>> + drm_mode_plane_set_obj_prop(plane, >> >>> + dev->mode_config.rotation_property, >> >>> + BIT(DRM_ROTATE_0)); >> >>> } >> >>> } >> >>> >> >>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h >> >>> index 0375d75..31344bf 100644 >> >>> --- a/include/drm/drm_crtc.h >> >>> +++ b/include/drm/drm_crtc.h >> >>> @@ -1127,6 +1127,9 @@ extern int drm_mode_obj_get_properties_ioctl(struct drm_device *dev, void *data, >> >>> struct drm_file *file_priv); >> >>> extern int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data, >> >>> struct drm_file *file_priv); >> >>> +extern int drm_mode_plane_set_obj_prop(struct drm_plane *plane, >> >>> + struct drm_property *property, >> >>> + uint64_t value); >> >>> >> >>> extern void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth, >> >>> int *bpp); >> >>> -- >> >>> 1.9.3 >> >>> >> >>> _______________________________________________ >> >>> Intel-gfx mailing list >> >>> Intel-gfx@lists.freedesktop.org >> >>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >> >> >> >> -- >> >> Daniel Vetter >> >> Software Engineer, Intel Corporation >> >> +41 (0) 79 365 57 48 - http://blog.ffwll.ch >> >> _______________________________________________ >> >> Intel-gfx mailing list >> >> Intel-gfx@lists.freedesktop.org >> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index f09b752..95f330a 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -4175,12 +4175,25 @@ static int drm_mode_crtc_set_obj_prop(struct drm_mode_object *obj, return ret; } -static int drm_mode_plane_set_obj_prop(struct drm_mode_object *obj, - struct drm_property *property, - uint64_t value) +/** + * drm_mode_plane_set_obj_prop - set the value of a property + * @plane: drm plane object to set property value for + * @property: property to set + * @val: value the property should be set to + * + * This functions sets a given property on a given plane object. This function + * calls the driver's ->set_property callback and changes the software state of + * the property if the callback succeeds. + * + * Returns: + * Zero on success, error code on failure. + */ +int drm_mode_plane_set_obj_prop(struct drm_plane *plane, + struct drm_property *property, + uint64_t value) { int ret = -EINVAL; - struct drm_plane *plane = obj_to_plane(obj); + struct drm_mode_object *obj = &plane->base; if (plane->funcs->set_property) ret = plane->funcs->set_property(plane, property, value); @@ -4189,6 +4202,7 @@ static int drm_mode_plane_set_obj_prop(struct drm_mode_object *obj, return ret; } +EXPORT_SYMBOL(drm_mode_plane_set_obj_prop); /** * drm_mode_getproperty_ioctl - get the current value of a object's property @@ -4327,7 +4341,8 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data, ret = drm_mode_crtc_set_obj_prop(arg_obj, property, arg->value); break; case DRM_MODE_OBJECT_PLANE: - ret = drm_mode_plane_set_obj_prop(arg_obj, property, arg->value); + ret = drm_mode_plane_set_obj_prop(obj_to_plane(arg_obj), + property, arg->value); break; } diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 63d7b8e..0c0c39b 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -296,9 +296,9 @@ static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper) drm_plane_force_disable(plane); if (dev->mode_config.rotation_property) { - drm_object_property_set_value(&plane->base, - dev->mode_config.rotation_property, - BIT(DRM_ROTATE_0)); + drm_mode_plane_set_obj_prop(plane, + dev->mode_config.rotation_property, + BIT(DRM_ROTATE_0)); } } diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 0375d75..31344bf 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1127,6 +1127,9 @@ extern int drm_mode_obj_get_properties_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); extern int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); +extern int drm_mode_plane_set_obj_prop(struct drm_plane *plane, + struct drm_property *property, + uint64_t value); extern void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth, int *bpp);
Make sure plane rotation is reset correctly when restoring the fbdev configuration by using drm_mode_plane_set_obj_prop. This calls the driver's set_property callback and ensures the rotation is actually changed. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82236 Signed-off-by: Thomas Wood <thomas.wood@intel.com> --- drivers/gpu/drm/drm_crtc.c | 25 ++++++++++++++++++++----- drivers/gpu/drm/drm_fb_helper.c | 6 +++--- include/drm/drm_crtc.h | 3 +++ 3 files changed, 26 insertions(+), 8 deletions(-)