All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm: Wrap the check for atomic_commit implementation
@ 2016-12-21  1:03 Dhinakaran Pandiyan
  2016-12-21  1:03 ` [PATCH 2/2] drm: Get atomic property value even if DRIVER_ATOMIC is not set Dhinakaran Pandiyan
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Dhinakaran Pandiyan @ 2016-12-21  1:03 UTC (permalink / raw)
  To: dri-devel; +Cc: Daniel Vetter, intel-gfx, Dhinakaran Pandiyan, Ben Skeggs

This check is useful for drivers that do not have DRIVER_ATOMIC set but
have atomic modesetting internally implemented. Wrap the check into a
function since this is used in many places and as a bonus, the function
name helps to document what the check is for.

Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ben Skeggs <bskeggs@redhat.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/drm_fb_helper.c             | 6 +++---
 drivers/gpu/drm/nouveau/nouveau_connector.c | 4 ++--
 drivers/gpu/drm/nouveau/nouveau_display.c   | 6 +++---
 drivers/gpu/drm/nouveau/nouveau_fbcon.c     | 2 +-
 include/drm/drmP.h                          | 5 +++++
 5 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 145d55f..730342c 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -405,7 +405,7 @@ static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
 
 	drm_warn_on_modeset_not_all_locked(dev);
 
-	if (dev->mode_config.funcs->atomic_commit)
+	if (drm_drv_uses_atomic_modeset(dev))
 		return restore_fbdev_mode_atomic(fb_helper);
 
 	drm_for_each_plane(plane, dev) {
@@ -1444,7 +1444,7 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
 		return -EBUSY;
 	}
 
-	if (dev->mode_config.funcs->atomic_commit) {
+	if (drm_drv_uses_atomic_modeset(dev)) {
 		ret = pan_display_atomic(var, info);
 		goto unlock;
 	}
@@ -2060,7 +2060,7 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
 	 * NULL we fallback to the default drm_atomic_helper_best_encoder()
 	 * helper.
 	 */
-	if (fb_helper->dev->mode_config.funcs->atomic_commit &&
+	if (drm_drv_uses_atomic_modeset(fb_helper->dev) &&
 	    !connector_funcs->best_encoder)
 		encoder = drm_atomic_helper_best_encoder(connector);
 	else
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 947c200..c4cc21a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -769,7 +769,7 @@ nouveau_connector_set_property(struct drm_connector *connector,
 	struct drm_encoder *encoder = to_drm_encoder(nv_encoder);
 	int ret;
 
-	if (connector->dev->mode_config.funcs->atomic_commit)
+	if (drm_drv_uses_atomic_modeset(connector->dev))
 		return drm_atomic_helper_connector_set_property(connector, property, value);
 
 	ret = connector->funcs->atomic_set_property(&nv_connector->base,
@@ -1074,7 +1074,7 @@ nouveau_connector_helper_funcs = {
 static int
 nouveau_connector_dpms(struct drm_connector *connector, int mode)
 {
-	if (connector->dev->mode_config.funcs->atomic_commit)
+	if (drm_drv_uses_atomic_modeset(connector->dev))
 		return drm_atomic_helper_connector_dpms(connector, mode);
 	return drm_helper_connector_dpms(connector, mode);
 }
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index c5cf888..add353e 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -162,7 +162,7 @@ nouveau_display_vblstamp(struct drm_device *dev, unsigned int pipe,
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
 		if (nouveau_crtc(crtc)->index == pipe) {
 			struct drm_display_mode *mode;
-			if (dev->mode_config.funcs->atomic_commit)
+			if (drm_drv_uses_atomic_modeset(dev))
 				mode = &crtc->state->adjusted_mode;
 			else
 				mode = &crtc->hwmode;
@@ -738,7 +738,7 @@ nouveau_display_suspend(struct drm_device *dev, bool runtime)
 	struct nouveau_display *disp = nouveau_display(dev);
 	struct drm_crtc *crtc;
 
-	if (dev->mode_config.funcs->atomic_commit) {
+	if (drm_drv_uses_atomic_modeset(dev)) {
 		if (!runtime) {
 			disp->suspend = nouveau_atomic_suspend(dev);
 			if (IS_ERR(disp->suspend)) {
@@ -784,7 +784,7 @@ nouveau_display_resume(struct drm_device *dev, bool runtime)
 	struct drm_crtc *crtc;
 	int ret;
 
-	if (dev->mode_config.funcs->atomic_commit) {
+	if (drm_drv_uses_atomic_modeset(dev)) {
 		nouveau_display_init(dev);
 		if (disp->suspend) {
 			drm_atomic_helper_resume(dev, disp->suspend);
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index 5600f6c..ad722f14 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -524,7 +524,7 @@ nouveau_fbcon_init(struct drm_device *dev)
 		preferred_bpp = 32;
 
 	/* disable all the possible outputs/crtcs before entering KMS mode */
-	if (!dev->mode_config.funcs->atomic_commit)
+	if (!drm_drv_uses_atomic_modeset(dev))
 		drm_helper_disable_unused_functions(dev);
 
 	ret = drm_fb_helper_initial_config(&fbcon->helper, preferred_bpp);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index a9cfd33..247acba 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -647,6 +647,11 @@ static __inline__ int drm_core_check_feature(struct drm_device *dev,
 	return ((dev->driver->driver_features & feature) ? 1 : 0);
 }
 
+static inline int drm_drv_uses_atomic_modeset(struct drm_device *dev)
+{
+	return dev->mode_config.funcs->atomic_commit ? 1 : 0;
+}
+
 static inline void drm_device_set_unplugged(struct drm_device *dev)
 {
 	smp_wmb();
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/2] drm: Get atomic property value even if DRIVER_ATOMIC is not set
  2016-12-21  1:03 [PATCH 1/2] drm: Wrap the check for atomic_commit implementation Dhinakaran Pandiyan
@ 2016-12-21  1:03 ` Dhinakaran Pandiyan
  2016-12-21  8:53   ` Daniel Vetter
  2016-12-21  1:45 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm: Wrap the check for atomic_commit implementation Patchwork
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Dhinakaran Pandiyan @ 2016-12-21  1:03 UTC (permalink / raw)
  To: dri-devel; +Cc: Daniel Vetter, intel-gfx, Dhinakaran Pandiyan

i915 does not set DRIVER_ATOMIC by default yet but uses atomic_check and
atomic_commit. drm_object_property_get_value() does not read the correct
value of atomic properties if DRIVER_ATOMIC is not set. Checking whether
the driver uses atomic modeset is a better check instead as the property
values are tracked in the state structures.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/drm_mode_object.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
index 9f17085..292d339 100644
--- a/drivers/gpu/drm/drm_mode_object.c
+++ b/drivers/gpu/drm/drm_mode_object.c
@@ -273,7 +273,7 @@ int drm_object_property_get_value(struct drm_mode_object *obj,
 	 * their value in obj->properties->values[].. mostly to avoid
 	 * having to deal w/ EDID and similar props in atomic paths:
 	 */
-	if (drm_core_check_feature(property->dev, DRIVER_ATOMIC) &&
+	if (drm_drv_uses_atomic_modeset(property->dev) &&
 			!(property->flags & DRM_MODE_PROP_IMMUTABLE))
 		return drm_atomic_get_property(obj, property, val);
 
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [1/2] drm: Wrap the check for atomic_commit implementation
  2016-12-21  1:03 [PATCH 1/2] drm: Wrap the check for atomic_commit implementation Dhinakaran Pandiyan
  2016-12-21  1:03 ` [PATCH 2/2] drm: Get atomic property value even if DRIVER_ATOMIC is not set Dhinakaran Pandiyan
@ 2016-12-21  1:45 ` Patchwork
  2016-12-21  8:51 ` [PATCH 1/2] " Daniel Vetter
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2016-12-21  1:45 UTC (permalink / raw)
  To: Pandiyan, Dhinakaran; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm: Wrap the check for atomic_commit implementation
URL   : https://patchwork.freedesktop.org/series/17075/
State : success

== Summary ==

Series 17075v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/17075/revisions/1/mbox/


fi-bdw-5557u     total:247  pass:233  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050     total:247  pass:208  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:247  pass:225  dwarn:1   dfail:0   fail:0   skip:21 
fi-bxt-t5700     total:247  pass:220  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-j1900     total:247  pass:220  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:247  pass:216  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:247  pass:228  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r     total:247  pass:228  dwarn:0   dfail:0   fail:0   skip:19 
fi-ilk-650       total:247  pass:195  dwarn:0   dfail:0   fail:0   skip:52 
fi-ivb-3520m     total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770      total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u     total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u     total:247  pass:234  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hq    total:247  pass:227  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k     total:247  pass:224  dwarn:3   dfail:0   fail:0   skip:20 
fi-skl-6770hq    total:247  pass:234  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m     total:247  pass:216  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600      total:247  pass:215  dwarn:0   dfail:0   fail:0   skip:32 

6096aee14ea52e3163729129ee7362e56ff3efb9 drm-tip: 2016y-12m-20d-16h-33m-17s UTC integration manifest
92a7917 drm: Get atomic property value even if DRIVER_ATOMIC is not set
4555b02 drm: Wrap the check for atomic_commit implementation

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3348/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] drm: Wrap the check for atomic_commit implementation
  2016-12-21  1:03 [PATCH 1/2] drm: Wrap the check for atomic_commit implementation Dhinakaran Pandiyan
  2016-12-21  1:03 ` [PATCH 2/2] drm: Get atomic property value even if DRIVER_ATOMIC is not set Dhinakaran Pandiyan
  2016-12-21  1:45 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm: Wrap the check for atomic_commit implementation Patchwork
@ 2016-12-21  8:51 ` Daniel Vetter
  2016-12-21 20:12   ` [PATCH v2 " Dhinakaran Pandiyan
  2016-12-21  9:23 ` [PATCH " Ville Syrjälä
  2016-12-21 20:31 ` ✗ Fi.CI.BAT: failure for series starting with [v2,2/2] drm: Get atomic property value even if DRIVER_ATOMIC is not set (rev3) Patchwork
  4 siblings, 1 reply; 16+ messages in thread
From: Daniel Vetter @ 2016-12-21  8:51 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: Daniel Vetter, intel-gfx, Ben Skeggs, dri-devel

On Tue, Dec 20, 2016 at 05:03:02PM -0800, Dhinakaran Pandiyan wrote:
> This check is useful for drivers that do not have DRIVER_ATOMIC set but
> have atomic modesetting internally implemented. Wrap the check into a
> function since this is used in many places and as a bonus, the function
> name helps to document what the check is for.
> 
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/drm_fb_helper.c             | 6 +++---
>  drivers/gpu/drm/nouveau/nouveau_connector.c | 4 ++--
>  drivers/gpu/drm/nouveau/nouveau_display.c   | 6 +++---
>  drivers/gpu/drm/nouveau/nouveau_fbcon.c     | 2 +-
>  include/drm/drmP.h                          | 5 +++++
>  5 files changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 145d55f..730342c 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -405,7 +405,7 @@ static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
>  
>  	drm_warn_on_modeset_not_all_locked(dev);
>  
> -	if (dev->mode_config.funcs->atomic_commit)
> +	if (drm_drv_uses_atomic_modeset(dev))
>  		return restore_fbdev_mode_atomic(fb_helper);
>  
>  	drm_for_each_plane(plane, dev) {
> @@ -1444,7 +1444,7 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
>  		return -EBUSY;
>  	}
>  
> -	if (dev->mode_config.funcs->atomic_commit) {
> +	if (drm_drv_uses_atomic_modeset(dev)) {
>  		ret = pan_display_atomic(var, info);
>  		goto unlock;
>  	}
> @@ -2060,7 +2060,7 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
>  	 * NULL we fallback to the default drm_atomic_helper_best_encoder()
>  	 * helper.
>  	 */
> -	if (fb_helper->dev->mode_config.funcs->atomic_commit &&
> +	if (drm_drv_uses_atomic_modeset(fb_helper->dev) &&
>  	    !connector_funcs->best_encoder)
>  		encoder = drm_atomic_helper_best_encoder(connector);
>  	else
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
> index 947c200..c4cc21a 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -769,7 +769,7 @@ nouveau_connector_set_property(struct drm_connector *connector,
>  	struct drm_encoder *encoder = to_drm_encoder(nv_encoder);
>  	int ret;
>  
> -	if (connector->dev->mode_config.funcs->atomic_commit)
> +	if (drm_drv_uses_atomic_modeset(connector->dev))
>  		return drm_atomic_helper_connector_set_property(connector, property, value);
>  
>  	ret = connector->funcs->atomic_set_property(&nv_connector->base,
> @@ -1074,7 +1074,7 @@ nouveau_connector_helper_funcs = {
>  static int
>  nouveau_connector_dpms(struct drm_connector *connector, int mode)
>  {
> -	if (connector->dev->mode_config.funcs->atomic_commit)
> +	if (drm_drv_uses_atomic_modeset(connector->dev))
>  		return drm_atomic_helper_connector_dpms(connector, mode);
>  	return drm_helper_connector_dpms(connector, mode);
>  }
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
> index c5cf888..add353e 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
> @@ -162,7 +162,7 @@ nouveau_display_vblstamp(struct drm_device *dev, unsigned int pipe,
>  	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
>  		if (nouveau_crtc(crtc)->index == pipe) {
>  			struct drm_display_mode *mode;
> -			if (dev->mode_config.funcs->atomic_commit)
> +			if (drm_drv_uses_atomic_modeset(dev))
>  				mode = &crtc->state->adjusted_mode;
>  			else
>  				mode = &crtc->hwmode;
> @@ -738,7 +738,7 @@ nouveau_display_suspend(struct drm_device *dev, bool runtime)
>  	struct nouveau_display *disp = nouveau_display(dev);
>  	struct drm_crtc *crtc;
>  
> -	if (dev->mode_config.funcs->atomic_commit) {
> +	if (drm_drv_uses_atomic_modeset(dev)) {
>  		if (!runtime) {
>  			disp->suspend = nouveau_atomic_suspend(dev);
>  			if (IS_ERR(disp->suspend)) {
> @@ -784,7 +784,7 @@ nouveau_display_resume(struct drm_device *dev, bool runtime)
>  	struct drm_crtc *crtc;
>  	int ret;
>  
> -	if (dev->mode_config.funcs->atomic_commit) {
> +	if (drm_drv_uses_atomic_modeset(dev)) {
>  		nouveau_display_init(dev);
>  		if (disp->suspend) {
>  			drm_atomic_helper_resume(dev, disp->suspend);
> diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
> index 5600f6c..ad722f14 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
> @@ -524,7 +524,7 @@ nouveau_fbcon_init(struct drm_device *dev)
>  		preferred_bpp = 32;
>  
>  	/* disable all the possible outputs/crtcs before entering KMS mode */
> -	if (!dev->mode_config.funcs->atomic_commit)
> +	if (!drm_drv_uses_atomic_modeset(dev))
>  		drm_helper_disable_unused_functions(dev);
>  
>  	ret = drm_fb_helper_initial_config(&fbcon->helper, preferred_bpp);
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index a9cfd33..247acba 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -647,6 +647,11 @@ static __inline__ int drm_core_check_feature(struct drm_device *dev,
>  	return ((dev->driver->driver_features & feature) ? 1 : 0);
>  }
>  
> +static inline int drm_drv_uses_atomic_modeset(struct drm_device *dev)

Please put this into the drm_atomic.h header and wrap some kernel-doc
around it to explain when to use this. Otherwise looks good to me.
-Daniel

> +{
> +	return dev->mode_config.funcs->atomic_commit ? 1 : 0;
> +}
> +
>  static inline void drm_device_set_unplugged(struct drm_device *dev)
>  {
>  	smp_wmb();
> -- 
> 2.7.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] drm: Get atomic property value even if DRIVER_ATOMIC is not set
  2016-12-21  1:03 ` [PATCH 2/2] drm: Get atomic property value even if DRIVER_ATOMIC is not set Dhinakaran Pandiyan
@ 2016-12-21  8:53   ` Daniel Vetter
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2016-12-21  8:53 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: Daniel Vetter, intel-gfx, dri-devel

On Tue, Dec 20, 2016 at 05:03:03PM -0800, Dhinakaran Pandiyan wrote:
> i915 does not set DRIVER_ATOMIC by default yet but uses atomic_check and
> atomic_commit. drm_object_property_get_value() does not read the correct
> value of atomic properties if DRIVER_ATOMIC is not set. Checking whether
> the driver uses atomic modeset is a better check instead as the property
> values are tracked in the state structures.
> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

I've reviewed all other users for DRIVER_ATOMIC checks, and I think this
is the only one left that we should change. Will merge as soon as you've
respun patch 1.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-Daniel
> ---
>  drivers/gpu/drm/drm_mode_object.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
> index 9f17085..292d339 100644
> --- a/drivers/gpu/drm/drm_mode_object.c
> +++ b/drivers/gpu/drm/drm_mode_object.c
> @@ -273,7 +273,7 @@ int drm_object_property_get_value(struct drm_mode_object *obj,
>  	 * their value in obj->properties->values[].. mostly to avoid
>  	 * having to deal w/ EDID and similar props in atomic paths:
>  	 */
> -	if (drm_core_check_feature(property->dev, DRIVER_ATOMIC) &&
> +	if (drm_drv_uses_atomic_modeset(property->dev) &&
>  			!(property->flags & DRM_MODE_PROP_IMMUTABLE))
>  		return drm_atomic_get_property(obj, property, val);
>  
> -- 
> 2.7.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] drm: Wrap the check for atomic_commit implementation
  2016-12-21  1:03 [PATCH 1/2] drm: Wrap the check for atomic_commit implementation Dhinakaran Pandiyan
                   ` (2 preceding siblings ...)
  2016-12-21  8:51 ` [PATCH 1/2] " Daniel Vetter
@ 2016-12-21  9:23 ` Ville Syrjälä
  2016-12-21 20:31 ` ✗ Fi.CI.BAT: failure for series starting with [v2,2/2] drm: Get atomic property value even if DRIVER_ATOMIC is not set (rev3) Patchwork
  4 siblings, 0 replies; 16+ messages in thread
From: Ville Syrjälä @ 2016-12-21  9:23 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: Daniel Vetter, intel-gfx, Ben Skeggs, dri-devel

On Tue, Dec 20, 2016 at 05:03:02PM -0800, Dhinakaran Pandiyan wrote:
> This check is useful for drivers that do not have DRIVER_ATOMIC set but
> have atomic modesetting internally implemented. Wrap the check into a
> function since this is used in many places and as a bonus, the function
> name helps to document what the check is for.
> 
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/drm_fb_helper.c             | 6 +++---
>  drivers/gpu/drm/nouveau/nouveau_connector.c | 4 ++--
>  drivers/gpu/drm/nouveau/nouveau_display.c   | 6 +++---
>  drivers/gpu/drm/nouveau/nouveau_fbcon.c     | 2 +-
>  include/drm/drmP.h                          | 5 +++++
>  5 files changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 145d55f..730342c 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -405,7 +405,7 @@ static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
>  
>  	drm_warn_on_modeset_not_all_locked(dev);
>  
> -	if (dev->mode_config.funcs->atomic_commit)
> +	if (drm_drv_uses_atomic_modeset(dev))
>  		return restore_fbdev_mode_atomic(fb_helper);
>  
>  	drm_for_each_plane(plane, dev) {
> @@ -1444,7 +1444,7 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
>  		return -EBUSY;
>  	}
>  
> -	if (dev->mode_config.funcs->atomic_commit) {
> +	if (drm_drv_uses_atomic_modeset(dev)) {
>  		ret = pan_display_atomic(var, info);
>  		goto unlock;
>  	}
> @@ -2060,7 +2060,7 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
>  	 * NULL we fallback to the default drm_atomic_helper_best_encoder()
>  	 * helper.
>  	 */
> -	if (fb_helper->dev->mode_config.funcs->atomic_commit &&
> +	if (drm_drv_uses_atomic_modeset(fb_helper->dev) &&
>  	    !connector_funcs->best_encoder)
>  		encoder = drm_atomic_helper_best_encoder(connector);
>  	else
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
> index 947c200..c4cc21a 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -769,7 +769,7 @@ nouveau_connector_set_property(struct drm_connector *connector,
>  	struct drm_encoder *encoder = to_drm_encoder(nv_encoder);
>  	int ret;
>  
> -	if (connector->dev->mode_config.funcs->atomic_commit)
> +	if (drm_drv_uses_atomic_modeset(connector->dev))
>  		return drm_atomic_helper_connector_set_property(connector, property, value);
>  
>  	ret = connector->funcs->atomic_set_property(&nv_connector->base,
> @@ -1074,7 +1074,7 @@ nouveau_connector_helper_funcs = {
>  static int
>  nouveau_connector_dpms(struct drm_connector *connector, int mode)
>  {
> -	if (connector->dev->mode_config.funcs->atomic_commit)
> +	if (drm_drv_uses_atomic_modeset(connector->dev))
>  		return drm_atomic_helper_connector_dpms(connector, mode);
>  	return drm_helper_connector_dpms(connector, mode);
>  }
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
> index c5cf888..add353e 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
> @@ -162,7 +162,7 @@ nouveau_display_vblstamp(struct drm_device *dev, unsigned int pipe,
>  	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
>  		if (nouveau_crtc(crtc)->index == pipe) {
>  			struct drm_display_mode *mode;
> -			if (dev->mode_config.funcs->atomic_commit)
> +			if (drm_drv_uses_atomic_modeset(dev))
>  				mode = &crtc->state->adjusted_mode;
>  			else
>  				mode = &crtc->hwmode;
> @@ -738,7 +738,7 @@ nouveau_display_suspend(struct drm_device *dev, bool runtime)
>  	struct nouveau_display *disp = nouveau_display(dev);
>  	struct drm_crtc *crtc;
>  
> -	if (dev->mode_config.funcs->atomic_commit) {
> +	if (drm_drv_uses_atomic_modeset(dev)) {
>  		if (!runtime) {
>  			disp->suspend = nouveau_atomic_suspend(dev);
>  			if (IS_ERR(disp->suspend)) {
> @@ -784,7 +784,7 @@ nouveau_display_resume(struct drm_device *dev, bool runtime)
>  	struct drm_crtc *crtc;
>  	int ret;
>  
> -	if (dev->mode_config.funcs->atomic_commit) {
> +	if (drm_drv_uses_atomic_modeset(dev)) {
>  		nouveau_display_init(dev);
>  		if (disp->suspend) {
>  			drm_atomic_helper_resume(dev, disp->suspend);
> diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
> index 5600f6c..ad722f14 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
> @@ -524,7 +524,7 @@ nouveau_fbcon_init(struct drm_device *dev)
>  		preferred_bpp = 32;
>  
>  	/* disable all the possible outputs/crtcs before entering KMS mode */
> -	if (!dev->mode_config.funcs->atomic_commit)
> +	if (!drm_drv_uses_atomic_modeset(dev))
>  		drm_helper_disable_unused_functions(dev);
>  
>  	ret = drm_fb_helper_initial_config(&fbcon->helper, preferred_bpp);
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index a9cfd33..247acba 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -647,6 +647,11 @@ static __inline__ int drm_core_check_feature(struct drm_device *dev,
>  	return ((dev->driver->driver_features & feature) ? 1 : 0);
>  }
>  
> +static inline int drm_drv_uses_atomic_modeset(struct drm_device *dev)
> +{
> +	return dev->mode_config.funcs->atomic_commit ? 1 : 0;
> +}

bool ...
{
	return atomic_commit != NULL;
}

> +
>  static inline void drm_device_set_unplugged(struct drm_device *dev)
>  {
>  	smp_wmb();
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 1/2] drm: Wrap the check for atomic_commit implementation
  2016-12-21  8:51 ` [PATCH 1/2] " Daniel Vetter
@ 2016-12-21 20:12   ` Dhinakaran Pandiyan
  2016-12-21 20:12     ` [PATCH v2 2/2] drm: Get atomic property value even if DRIVER_ATOMIC is not set Dhinakaran Pandiyan
                       ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Dhinakaran Pandiyan @ 2016-12-21 20:12 UTC (permalink / raw)
  To: intel-gfx
  Cc: Daniel Vetter, dri-devel, Ben Skeggs, nouveau, Dhinakaran Pandiyan

This check is useful for drivers that do not have DRIVER_ATOMIC set but
have atomic modesetting internally implemented. Wrap the check into a
function since this is used in many places and as a bonus, the function
name helps to document what the check is for.

v2:
Change return type to bool (Ville)
Move the function drm_atomic.h (Daniel)

Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ben Skeggs <bskeggs@redhat.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/drm_fb_helper.c             |  6 +++---
 drivers/gpu/drm/nouveau/nouveau_connector.c |  5 +++--
 drivers/gpu/drm/nouveau/nouveau_display.c   |  6 +++---
 drivers/gpu/drm/nouveau/nouveau_fbcon.c     |  3 ++-
 include/drm/drm_atomic.h                    | 11 +++++++++++
 5 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 145d55f..730342c 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -405,7 +405,7 @@ static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
 
 	drm_warn_on_modeset_not_all_locked(dev);
 
-	if (dev->mode_config.funcs->atomic_commit)
+	if (drm_drv_uses_atomic_modeset(dev))
 		return restore_fbdev_mode_atomic(fb_helper);
 
 	drm_for_each_plane(plane, dev) {
@@ -1444,7 +1444,7 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
 		return -EBUSY;
 	}
 
-	if (dev->mode_config.funcs->atomic_commit) {
+	if (drm_drv_uses_atomic_modeset(dev)) {
 		ret = pan_display_atomic(var, info);
 		goto unlock;
 	}
@@ -2060,7 +2060,7 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
 	 * NULL we fallback to the default drm_atomic_helper_best_encoder()
 	 * helper.
 	 */
-	if (fb_helper->dev->mode_config.funcs->atomic_commit &&
+	if (drm_drv_uses_atomic_modeset(fb_helper->dev) &&
 	    !connector_funcs->best_encoder)
 		encoder = drm_atomic_helper_best_encoder(connector);
 	else
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 947c200..966d20a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -33,6 +33,7 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_edid.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_atomic.h>
 
 #include "nouveau_reg.h"
 #include "nouveau_drv.h"
@@ -769,7 +770,7 @@ nouveau_connector_set_property(struct drm_connector *connector,
 	struct drm_encoder *encoder = to_drm_encoder(nv_encoder);
 	int ret;
 
-	if (connector->dev->mode_config.funcs->atomic_commit)
+	if (drm_drv_uses_atomic_modeset(connector->dev))
 		return drm_atomic_helper_connector_set_property(connector, property, value);
 
 	ret = connector->funcs->atomic_set_property(&nv_connector->base,
@@ -1074,7 +1075,7 @@ nouveau_connector_helper_funcs = {
 static int
 nouveau_connector_dpms(struct drm_connector *connector, int mode)
 {
-	if (connector->dev->mode_config.funcs->atomic_commit)
+	if (drm_drv_uses_atomic_modeset(connector->dev))
 		return drm_atomic_helper_connector_dpms(connector, mode);
 	return drm_helper_connector_dpms(connector, mode);
 }
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index c5cf888..add353e 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -162,7 +162,7 @@ nouveau_display_vblstamp(struct drm_device *dev, unsigned int pipe,
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
 		if (nouveau_crtc(crtc)->index == pipe) {
 			struct drm_display_mode *mode;
-			if (dev->mode_config.funcs->atomic_commit)
+			if (drm_drv_uses_atomic_modeset(dev))
 				mode = &crtc->state->adjusted_mode;
 			else
 				mode = &crtc->hwmode;
@@ -738,7 +738,7 @@ nouveau_display_suspend(struct drm_device *dev, bool runtime)
 	struct nouveau_display *disp = nouveau_display(dev);
 	struct drm_crtc *crtc;
 
-	if (dev->mode_config.funcs->atomic_commit) {
+	if (drm_drv_uses_atomic_modeset(dev)) {
 		if (!runtime) {
 			disp->suspend = nouveau_atomic_suspend(dev);
 			if (IS_ERR(disp->suspend)) {
@@ -784,7 +784,7 @@ nouveau_display_resume(struct drm_device *dev, bool runtime)
 	struct drm_crtc *crtc;
 	int ret;
 
-	if (dev->mode_config.funcs->atomic_commit) {
+	if (drm_drv_uses_atomic_modeset(dev)) {
 		nouveau_display_init(dev);
 		if (disp->suspend) {
 			drm_atomic_helper_resume(dev, disp->suspend);
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index 5600f6c..9de6abb 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -41,6 +41,7 @@
 #include <drm/drm_crtc.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fb_helper.h>
+#include <drm/drm_atomic.h>
 
 #include "nouveau_drv.h"
 #include "nouveau_gem.h"
@@ -524,7 +525,7 @@ nouveau_fbcon_init(struct drm_device *dev)
 		preferred_bpp = 32;
 
 	/* disable all the possible outputs/crtcs before entering KMS mode */
-	if (!dev->mode_config.funcs->atomic_commit)
+	if (!drm_drv_uses_atomic_modeset(dev))
 		drm_helper_disable_unused_functions(dev);
 
 	ret = drm_fb_helper_initial_config(&fbcon->helper, preferred_bpp);
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index 8cc7ca2..43db162 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -419,5 +419,16 @@ drm_atomic_crtc_needs_modeset(const struct drm_crtc_state *state)
 	       state->connectors_changed;
 }
 
+/* drm_drv_uses_atomic_modeset - check if the driver implements
+ * atomic_commit()
+ * @dev: DRM device
+ *
+ * This check is useful if drivers do not have DRIVER_ATOMIC set but
+ * have atomic modesetting internally implemented.
+ */
+static inline bool drm_drv_uses_atomic_modeset(struct drm_device *dev)
+{
+	return dev->mode_config.funcs->atomic_commit ? true : false;
+}
 
 #endif /* DRM_ATOMIC_H_ */
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 2/2] drm: Get atomic property value even if DRIVER_ATOMIC is not set
  2016-12-21 20:12   ` [PATCH v2 " Dhinakaran Pandiyan
@ 2016-12-21 20:12     ` Dhinakaran Pandiyan
  2017-01-03 16:06       ` Alex Deucher
  2016-12-22  7:11     ` [PATCH v2 1/2] drm: Wrap the check for atomic_commit implementation Daniel Vetter
                       ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ messages in thread
From: Dhinakaran Pandiyan @ 2016-12-21 20:12 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter, nouveau, dri-devel, Dhinakaran Pandiyan

i915 does not set DRIVER_ATOMIC by default yet but uses atomic_check and
atomic_commit. drm_object_property_get_value() does not read the correct
value of atomic properties if DRIVER_ATOMIC is not set. Checking whether
the driver uses atomic modeset is a better check instead as the property
values are tracked in the state structures.

v2: Included header

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/drm_mode_object.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
index 9f17085..14543ff 100644
--- a/drivers/gpu/drm/drm_mode_object.c
+++ b/drivers/gpu/drm/drm_mode_object.c
@@ -23,6 +23,7 @@
 #include <linux/export.h>
 #include <drm/drmP.h>
 #include <drm/drm_mode_object.h>
+#include <drm/drm_atomic.h>
 
 #include "drm_crtc_internal.h"
 
@@ -273,7 +274,7 @@ int drm_object_property_get_value(struct drm_mode_object *obj,
 	 * their value in obj->properties->values[].. mostly to avoid
 	 * having to deal w/ EDID and similar props in atomic paths:
 	 */
-	if (drm_core_check_feature(property->dev, DRIVER_ATOMIC) &&
+	if (drm_drv_uses_atomic_modeset(property->dev) &&
 			!(property->flags & DRM_MODE_PROP_IMMUTABLE))
 		return drm_atomic_get_property(obj, property, val);
 
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* ✗ Fi.CI.BAT: failure for series starting with [v2,2/2] drm: Get atomic property value even if DRIVER_ATOMIC is not set (rev3)
  2016-12-21  1:03 [PATCH 1/2] drm: Wrap the check for atomic_commit implementation Dhinakaran Pandiyan
                   ` (3 preceding siblings ...)
  2016-12-21  9:23 ` [PATCH " Ville Syrjälä
@ 2016-12-21 20:31 ` Patchwork
  2016-12-21 22:14   ` Pandiyan, Dhinakaran
  4 siblings, 1 reply; 16+ messages in thread
From: Patchwork @ 2016-12-21 20:31 UTC (permalink / raw)
  To: Pandiyan, Dhinakaran; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,2/2] drm: Get atomic property value even if DRIVER_ATOMIC is not set (rev3)
URL   : https://patchwork.freedesktop.org/series/17075/
State : failure

== Summary ==

  CC [M]  drivers/gpu/drm/i915/intel_dsi_panel_vbt.o
  CC [M]  drivers/gpu/drm/i915/intel_dsi_pll.o
  CC [M]  drivers/gpu/drm/i915/intel_i2c.o
  CC [M]  drivers/gpu/drm/i915/intel_hdmi.o
  CC [M]  drivers/gpu/drm/i915/intel_dvo.o
  CC [M]  drivers/gpu/drm/i915/intel_lvds.o
  CC [M]  drivers/gpu/drm/i915/intel_lspcon.o
  CC [M]  drivers/gpu/drm/i915/intel_panel.o
  CC [M]  drivers/gpu/drm/i915/intel_sdvo.o
  CC [M]  drivers/gpu/drm/i915/intel_tv.o
  CC [M]  drivers/gpu/drm/i915/i915_perf.o
  CC [M]  drivers/gpu/drm/i915/i915_vgpu.o
  CC [M]  drivers/gpu/drm/i915/i915_gpu_error.o
  CC [M]  drivers/gpu/drm/i915/intel_gvt.o
  CC [M]  drivers/gpu/drm/i915/i915_oa_hsw.o
  CC [M]  drivers/gpu/drm/i915/gvt/gvt.o
  CC [M]  drivers/gpu/drm/i915/gvt/aperture_gm.o
  CC [M]  drivers/gpu/drm/i915/gvt/handlers.o
  CC [M]  drivers/gpu/drm/i915/gvt/trace_points.o
  CC [M]  drivers/gpu/drm/i915/gvt/vgpu.o
  CC [M]  drivers/gpu/drm/i915/gvt/firmware.o
  CC [M]  drivers/gpu/drm/i915/gvt/interrupt.o
  CC [M]  drivers/gpu/drm/i915/gvt/gtt.o
  CC [M]  drivers/gpu/drm/i915/gvt/cfg_space.o
  CC [M]  drivers/gpu/drm/i915/gvt/mmio.o
  CC [M]  drivers/gpu/drm/i915/gvt/opregion.o
  CC [M]  drivers/gpu/drm/i915/gvt/display.o
  CC [M]  drivers/gpu/drm/i915/gvt/edid.o
  CC [M]  drivers/gpu/drm/i915/gvt/execlist.o
  CC [M]  drivers/gpu/drm/i915/gvt/sched_policy.o
  CC [M]  drivers/gpu/drm/i915/gvt/render.o
  CC [M]  drivers/gpu/drm/i915/gvt/scheduler.o
  CC [M]  drivers/gpu/drm/i915/gvt/cmd_parser.o
  LD [M]  sound/pci/hda/snd-hda-codec-generic.o
  LD      sound/pci/built-in.o
  LD      drivers/thermal/thermal_sys.o
  LD      drivers/thermal/built-in.o
  LD      drivers/spi/built-in.o
  LD      drivers/acpi/acpica/acpi.o
  LD      drivers/video/fbdev/built-in.o
  LD      drivers/tty/serial/8250/8250.o
  LD [M]  drivers/net/ethernet/intel/igbvf/igbvf.o
  LD      sound/built-in.o
  LD      drivers/iommu/built-in.o
  LD      net/packet/built-in.o
  LD      drivers/acpi/acpica/built-in.o
  LD      drivers/usb/gadget/udc/udc-core.o
  LD      drivers/scsi/scsi_mod.o
  LD      drivers/usb/gadget/udc/built-in.o
  LD      drivers/usb/gadget/built-in.o
  LD      drivers/acpi/built-in.o
  LD [M]  drivers/net/ethernet/intel/e1000/e1000.o
  LD      net/xfrm/built-in.o
  LD      lib/raid6/raid6_pq.o
  LD [M]  drivers/net/ethernet/broadcom/genet/genet.o
  LD      drivers/video/console/built-in.o
  LD      lib/raid6/built-in.o
  LD      drivers/video/built-in.o
  LD      drivers/tty/serial/8250/8250_base.o
  LD      drivers/scsi/sd_mod.o
  LD      drivers/tty/serial/8250/built-in.o
  LD      drivers/scsi/built-in.o
  LD      drivers/tty/serial/built-in.o
  LD      net/ipv6/ipv6.o
  LD      drivers/usb/core/usbcore.o
  LD [M]  drivers/net/ethernet/intel/igb/igb.o
  LD      net/ipv6/built-in.o
  LD      drivers/usb/core/built-in.o
  AR      lib/lib.a
  EXPORTS lib/lib-ksyms.o
  LD      lib/built-in.o
  LD      fs/btrfs/btrfs.o
  LD      drivers/md/md-mod.o
  LD      drivers/md/built-in.o
  CC      arch/x86/kernel/cpu/capflags.o
  LD      arch/x86/kernel/cpu/built-in.o
  LD      arch/x86/kernel/built-in.o
  LD      fs/btrfs/built-in.o
  LD      drivers/tty/vt/built-in.o
  LD      drivers/tty/built-in.o
  LD      arch/x86/built-in.o
  LD      net/ipv4/built-in.o
  LD      drivers/usb/host/xhci-hcd.o
  LD      drivers/usb/host/built-in.o
  LD      drivers/usb/built-in.o
  LD [M]  drivers/net/ethernet/intel/e1000e/e1000e.o
  LD      net/core/built-in.o
  LD      net/built-in.o
  LD      fs/ext4/ext4.o
  LD      fs/ext4/built-in.o
  LD      fs/built-in.o
  LD      drivers/net/ethernet/built-in.o
  LD      drivers/net/built-in.o
  LD [M]  drivers/gpu/drm/i915/i915.o
scripts/Makefile.build:544: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:544: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:988: recipe for target 'drivers' failed
make: *** [drivers] Error 2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: ✗ Fi.CI.BAT: failure for series starting with [v2,2/2] drm: Get atomic property value even if DRIVER_ATOMIC is not set (rev3)
  2016-12-21 20:31 ` ✗ Fi.CI.BAT: failure for series starting with [v2,2/2] drm: Get atomic property value even if DRIVER_ATOMIC is not set (rev3) Patchwork
@ 2016-12-21 22:14   ` Pandiyan, Dhinakaran
  0 siblings, 0 replies; 16+ messages in thread
From: Pandiyan, Dhinakaran @ 2016-12-21 22:14 UTC (permalink / raw)
  To: intel-gfx; +Cc: Pandiyan

Don't know what went wrong here, builds fine on my local machine.

-DK

On Wed, 2016-12-21 at 20:31 +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [v2,2/2] drm: Get atomic property value even if DRIVER_ATOMIC is not set (rev3)
> URL   : https://patchwork.freedesktop.org/series/17075/
> State : failure
> 
> == Summary ==
> 
>   CC [M]  drivers/gpu/drm/i915/intel_dsi_panel_vbt.o
>   CC [M]  drivers/gpu/drm/i915/intel_dsi_pll.o
>   CC [M]  drivers/gpu/drm/i915/intel_i2c.o
>   CC [M]  drivers/gpu/drm/i915/intel_hdmi.o
>   CC [M]  drivers/gpu/drm/i915/intel_dvo.o
>   CC [M]  drivers/gpu/drm/i915/intel_lvds.o
>   CC [M]  drivers/gpu/drm/i915/intel_lspcon.o
>   CC [M]  drivers/gpu/drm/i915/intel_panel.o
>   CC [M]  drivers/gpu/drm/i915/intel_sdvo.o
>   CC [M]  drivers/gpu/drm/i915/intel_tv.o
>   CC [M]  drivers/gpu/drm/i915/i915_perf.o
>   CC [M]  drivers/gpu/drm/i915/i915_vgpu.o
>   CC [M]  drivers/gpu/drm/i915/i915_gpu_error.o
>   CC [M]  drivers/gpu/drm/i915/intel_gvt.o
>   CC [M]  drivers/gpu/drm/i915/i915_oa_hsw.o
>   CC [M]  drivers/gpu/drm/i915/gvt/gvt.o
>   CC [M]  drivers/gpu/drm/i915/gvt/aperture_gm.o
>   CC [M]  drivers/gpu/drm/i915/gvt/handlers.o
>   CC [M]  drivers/gpu/drm/i915/gvt/trace_points.o
>   CC [M]  drivers/gpu/drm/i915/gvt/vgpu.o
>   CC [M]  drivers/gpu/drm/i915/gvt/firmware.o
>   CC [M]  drivers/gpu/drm/i915/gvt/interrupt.o
>   CC [M]  drivers/gpu/drm/i915/gvt/gtt.o
>   CC [M]  drivers/gpu/drm/i915/gvt/cfg_space.o
>   CC [M]  drivers/gpu/drm/i915/gvt/mmio.o
>   CC [M]  drivers/gpu/drm/i915/gvt/opregion.o
>   CC [M]  drivers/gpu/drm/i915/gvt/display.o
>   CC [M]  drivers/gpu/drm/i915/gvt/edid.o
>   CC [M]  drivers/gpu/drm/i915/gvt/execlist.o
>   CC [M]  drivers/gpu/drm/i915/gvt/sched_policy.o
>   CC [M]  drivers/gpu/drm/i915/gvt/render.o
>   CC [M]  drivers/gpu/drm/i915/gvt/scheduler.o
>   CC [M]  drivers/gpu/drm/i915/gvt/cmd_parser.o
>   LD [M]  sound/pci/hda/snd-hda-codec-generic.o
>   LD      sound/pci/built-in.o
>   LD      drivers/thermal/thermal_sys.o
>   LD      drivers/thermal/built-in.o
>   LD      drivers/spi/built-in.o
>   LD      drivers/acpi/acpica/acpi.o
>   LD      drivers/video/fbdev/built-in.o
>   LD      drivers/tty/serial/8250/8250.o
>   LD [M]  drivers/net/ethernet/intel/igbvf/igbvf.o
>   LD      sound/built-in.o
>   LD      drivers/iommu/built-in.o
>   LD      net/packet/built-in.o
>   LD      drivers/acpi/acpica/built-in.o
>   LD      drivers/usb/gadget/udc/udc-core.o
>   LD      drivers/scsi/scsi_mod.o
>   LD      drivers/usb/gadget/udc/built-in.o
>   LD      drivers/usb/gadget/built-in.o
>   LD      drivers/acpi/built-in.o
>   LD [M]  drivers/net/ethernet/intel/e1000/e1000.o
>   LD      net/xfrm/built-in.o
>   LD      lib/raid6/raid6_pq.o
>   LD [M]  drivers/net/ethernet/broadcom/genet/genet.o
>   LD      drivers/video/console/built-in.o
>   LD      lib/raid6/built-in.o
>   LD      drivers/video/built-in.o
>   LD      drivers/tty/serial/8250/8250_base.o
>   LD      drivers/scsi/sd_mod.o
>   LD      drivers/tty/serial/8250/built-in.o
>   LD      drivers/scsi/built-in.o
>   LD      drivers/tty/serial/built-in.o
>   LD      net/ipv6/ipv6.o
>   LD      drivers/usb/core/usbcore.o
>   LD [M]  drivers/net/ethernet/intel/igb/igb.o
>   LD      net/ipv6/built-in.o
>   LD      drivers/usb/core/built-in.o
>   AR      lib/lib.a
>   EXPORTS lib/lib-ksyms.o
>   LD      lib/built-in.o
>   LD      fs/btrfs/btrfs.o
>   LD      drivers/md/md-mod.o
>   LD      drivers/md/built-in.o
>   CC      arch/x86/kernel/cpu/capflags.o
>   LD      arch/x86/kernel/cpu/built-in.o
>   LD      arch/x86/kernel/built-in.o
>   LD      fs/btrfs/built-in.o
>   LD      drivers/tty/vt/built-in.o
>   LD      drivers/tty/built-in.o
>   LD      arch/x86/built-in.o
>   LD      net/ipv4/built-in.o
>   LD      drivers/usb/host/xhci-hcd.o
>   LD      drivers/usb/host/built-in.o
>   LD      drivers/usb/built-in.o
>   LD [M]  drivers/net/ethernet/intel/e1000e/e1000e.o
>   LD      net/core/built-in.o
>   LD      net/built-in.o
>   LD      fs/ext4/ext4.o
>   LD      fs/ext4/built-in.o
>   LD      fs/built-in.o
>   LD      drivers/net/ethernet/built-in.o
>   LD      drivers/net/built-in.o
>   LD [M]  drivers/gpu/drm/i915/i915.o
> scripts/Makefile.build:544: recipe for target 'drivers/gpu/drm' failed
> make[2]: *** [drivers/gpu/drm] Error 2
> scripts/Makefile.build:544: recipe for target 'drivers/gpu' failed
> make[1]: *** [drivers/gpu] Error 2
> Makefile:988: recipe for target 'drivers' failed
> make: *** [drivers] Error 2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/2] drm: Wrap the check for atomic_commit implementation
  2016-12-21 20:12   ` [PATCH v2 " Dhinakaran Pandiyan
  2016-12-21 20:12     ` [PATCH v2 2/2] drm: Get atomic property value even if DRIVER_ATOMIC is not set Dhinakaran Pandiyan
@ 2016-12-22  7:11     ` Daniel Vetter
  2016-12-22  8:07     ` kbuild test robot
  2016-12-22  8:36     ` [Intel-gfx] " Ander Conselvan De Oliveira
  3 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2016-12-22  7:11 UTC (permalink / raw)
  To: Dhinakaran Pandiyan
  Cc: nouveau, intel-gfx, dri-devel, Ben Skeggs, Daniel Vetter

On Wed, Dec 21, 2016 at 12:12:08PM -0800, Dhinakaran Pandiyan wrote:
> This check is useful for drivers that do not have DRIVER_ATOMIC set but
> have atomic modesetting internally implemented. Wrap the check into a
> function since this is used in many places and as a bonus, the function
> name helps to document what the check is for.
> 
> v2:
> Change return type to bool (Ville)
> Move the function drm_atomic.h (Daniel)
> 
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/drm_fb_helper.c             |  6 +++---
>  drivers/gpu/drm/nouveau/nouveau_connector.c |  5 +++--
>  drivers/gpu/drm/nouveau/nouveau_display.c   |  6 +++---
>  drivers/gpu/drm/nouveau/nouveau_fbcon.c     |  3 ++-
>  include/drm/drm_atomic.h                    | 11 +++++++++++
>  5 files changed, 22 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 145d55f..730342c 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -405,7 +405,7 @@ static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
>  
>  	drm_warn_on_modeset_not_all_locked(dev);
>  
> -	if (dev->mode_config.funcs->atomic_commit)
> +	if (drm_drv_uses_atomic_modeset(dev))
>  		return restore_fbdev_mode_atomic(fb_helper);
>  
>  	drm_for_each_plane(plane, dev) {
> @@ -1444,7 +1444,7 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
>  		return -EBUSY;
>  	}
>  
> -	if (dev->mode_config.funcs->atomic_commit) {
> +	if (drm_drv_uses_atomic_modeset(dev)) {
>  		ret = pan_display_atomic(var, info);
>  		goto unlock;
>  	}
> @@ -2060,7 +2060,7 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
>  	 * NULL we fallback to the default drm_atomic_helper_best_encoder()
>  	 * helper.
>  	 */
> -	if (fb_helper->dev->mode_config.funcs->atomic_commit &&
> +	if (drm_drv_uses_atomic_modeset(fb_helper->dev) &&
>  	    !connector_funcs->best_encoder)
>  		encoder = drm_atomic_helper_best_encoder(connector);
>  	else
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
> index 947c200..966d20a 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -33,6 +33,7 @@
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_edid.h>
>  #include <drm/drm_crtc_helper.h>
> +#include <drm/drm_atomic.h>
>  
>  #include "nouveau_reg.h"
>  #include "nouveau_drv.h"
> @@ -769,7 +770,7 @@ nouveau_connector_set_property(struct drm_connector *connector,
>  	struct drm_encoder *encoder = to_drm_encoder(nv_encoder);
>  	int ret;
>  
> -	if (connector->dev->mode_config.funcs->atomic_commit)
> +	if (drm_drv_uses_atomic_modeset(connector->dev))
>  		return drm_atomic_helper_connector_set_property(connector, property, value);
>  
>  	ret = connector->funcs->atomic_set_property(&nv_connector->base,
> @@ -1074,7 +1075,7 @@ nouveau_connector_helper_funcs = {
>  static int
>  nouveau_connector_dpms(struct drm_connector *connector, int mode)
>  {
> -	if (connector->dev->mode_config.funcs->atomic_commit)
> +	if (drm_drv_uses_atomic_modeset(connector->dev))
>  		return drm_atomic_helper_connector_dpms(connector, mode);
>  	return drm_helper_connector_dpms(connector, mode);
>  }
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
> index c5cf888..add353e 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
> @@ -162,7 +162,7 @@ nouveau_display_vblstamp(struct drm_device *dev, unsigned int pipe,
>  	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
>  		if (nouveau_crtc(crtc)->index == pipe) {
>  			struct drm_display_mode *mode;
> -			if (dev->mode_config.funcs->atomic_commit)
> +			if (drm_drv_uses_atomic_modeset(dev))
>  				mode = &crtc->state->adjusted_mode;
>  			else
>  				mode = &crtc->hwmode;
> @@ -738,7 +738,7 @@ nouveau_display_suspend(struct drm_device *dev, bool runtime)
>  	struct nouveau_display *disp = nouveau_display(dev);
>  	struct drm_crtc *crtc;
>  
> -	if (dev->mode_config.funcs->atomic_commit) {
> +	if (drm_drv_uses_atomic_modeset(dev)) {
>  		if (!runtime) {
>  			disp->suspend = nouveau_atomic_suspend(dev);
>  			if (IS_ERR(disp->suspend)) {
> @@ -784,7 +784,7 @@ nouveau_display_resume(struct drm_device *dev, bool runtime)
>  	struct drm_crtc *crtc;
>  	int ret;
>  
> -	if (dev->mode_config.funcs->atomic_commit) {
> +	if (drm_drv_uses_atomic_modeset(dev)) {
>  		nouveau_display_init(dev);
>  		if (disp->suspend) {
>  			drm_atomic_helper_resume(dev, disp->suspend);
> diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
> index 5600f6c..9de6abb 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
> @@ -41,6 +41,7 @@
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_crtc_helper.h>
>  #include <drm/drm_fb_helper.h>
> +#include <drm/drm_atomic.h>
>  
>  #include "nouveau_drv.h"
>  #include "nouveau_gem.h"
> @@ -524,7 +525,7 @@ nouveau_fbcon_init(struct drm_device *dev)
>  		preferred_bpp = 32;
>  
>  	/* disable all the possible outputs/crtcs before entering KMS mode */
> -	if (!dev->mode_config.funcs->atomic_commit)
> +	if (!drm_drv_uses_atomic_modeset(dev))
>  		drm_helper_disable_unused_functions(dev);
>  
>  	ret = drm_fb_helper_initial_config(&fbcon->helper, preferred_bpp);
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index 8cc7ca2..43db162 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -419,5 +419,16 @@ drm_atomic_crtc_needs_modeset(const struct drm_crtc_state *state)
>  	       state->connectors_changed;
>  }
>  
> +/* drm_drv_uses_atomic_modeset - check if the driver implements
> + * atomic_commit()
> + * @dev: DRM device
> + *
> + * This check is useful if drivers do not have DRIVER_ATOMIC set but
> + * have atomic modesetting internally implemented.
> + */
> +static inline bool drm_drv_uses_atomic_modeset(struct drm_device *dev)
> +{
> +	return dev->mode_config.funcs->atomic_commit ? true : false;

You forgot the part from Ville's review that the entire "?:" at the end is
redundant: A pointer in bool context automatically converts to a bool like
this, and we use this _everywhere_. Please remove and respin.

Also when you resend the entire patch series, pls start a new thread. CI
otherwise assumes that you're only partially resend patches and tries to
merge old&new series, which doesn't apply ofc. Since it's already a mess
you new to start a new series even if only patch 1 changed.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/2] drm: Wrap the check for atomic_commit implementation
  2016-12-21 20:12   ` [PATCH v2 " Dhinakaran Pandiyan
  2016-12-21 20:12     ` [PATCH v2 2/2] drm: Get atomic property value even if DRIVER_ATOMIC is not set Dhinakaran Pandiyan
  2016-12-22  7:11     ` [PATCH v2 1/2] drm: Wrap the check for atomic_commit implementation Daniel Vetter
@ 2016-12-22  8:07     ` kbuild test robot
  2016-12-22  8:36     ` [Intel-gfx] " Ander Conselvan De Oliveira
  3 siblings, 0 replies; 16+ messages in thread
From: kbuild test robot @ 2016-12-22  8:07 UTC (permalink / raw)
  Cc: nouveau, intel-gfx, dri-devel, Dhinakaran Pandiyan, kbuild-all,
	Daniel Vetter, Ben Skeggs

[-- Attachment #1: Type: text/plain, Size: 1554 bytes --]

Hi Dhinakaran,

[auto build test ERROR on drm/drm-next]
[also build test ERROR on next-20161222]
[cannot apply to v4.9]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Dhinakaran-Pandiyan/drm-Wrap-the-check-for-atomic_commit-implementation/20161222-153523
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
config: i386-randconfig-s1-201651 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   In file included from drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c:19:0:
   include/drm/drm_atomic.h: In function 'drm_drv_uses_atomic_modeset':
>> include/drm/drm_atomic.h:436:12: error: dereferencing pointer to incomplete type 'struct drm_device'
     return dev->mode_config.funcs->atomic_commit ? true : false;
               ^~

vim +436 include/drm/drm_atomic.h

   430	 *
   431	 * This check is useful if drivers do not have DRIVER_ATOMIC set but
   432	 * have atomic modesetting internally implemented.
   433	 */
   434	static inline bool drm_drv_uses_atomic_modeset(struct drm_device *dev)
   435	{
 > 436		return dev->mode_config.funcs->atomic_commit ? true : false;
   437	}
   438	
   439	#endif /* DRM_ATOMIC_H_ */

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33192 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Intel-gfx] [PATCH v2 1/2] drm: Wrap the check for atomic_commit implementation
  2016-12-21 20:12   ` [PATCH v2 " Dhinakaran Pandiyan
                       ` (2 preceding siblings ...)
  2016-12-22  8:07     ` kbuild test robot
@ 2016-12-22  8:36     ` Ander Conselvan De Oliveira
       [not found]       ` <1482395761.4759.32.camel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  3 siblings, 1 reply; 16+ messages in thread
From: Ander Conselvan De Oliveira @ 2016-12-22  8:36 UTC (permalink / raw)
  To: Dhinakaran Pandiyan, intel-gfx
  Cc: Daniel Vetter, Ben Skeggs, dri-devel, nouveau

On Wed, 2016-12-21 at 12:12 -0800, Dhinakaran Pandiyan wrote:
> This check is useful for drivers that do not have DRIVER_ATOMIC set but
> have atomic modesetting internally implemented. Wrap the check into a
> function since this is used in many places and as a bonus, the function
> name helps to document what the check is for.
> 
> v2:
> Change return type to bool (Ville)
> Move the function drm_atomic.h (Daniel)
> 
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/drm_fb_helper.c             |  6 +++---
>  drivers/gpu/drm/nouveau/nouveau_connector.c |  5 +++--
>  drivers/gpu/drm/nouveau/nouveau_display.c   |  6 +++---
>  drivers/gpu/drm/nouveau/nouveau_fbcon.c     |  3 ++-
>  include/drm/drm_atomic.h                    | 11 +++++++++++
>  5 files changed, 22 insertions(+), 9 deletions(-)
> 

...

> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index 8cc7ca2..43db162 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -419,5 +419,16 @@ drm_atomic_crtc_needs_modeset(const struct drm_crtc_state
> *state)
>  	       state->connectors_changed;
>  }
>  
> +/* drm_drv_uses_atomic_modeset - check if the driver implements

Shouldn't this be

/**
 * drm_drv_uses_atomic_modeset - ...

so it is included in the generated documentation?

Ander


> + * atomic_commit()
> + * @dev: DRM device
> + *
> + * This check is useful if drivers do not have DRIVER_ATOMIC set but
> + * have atomic modesetting internally implemented.
> + */
> +static inline bool drm_drv_uses_atomic_modeset(struct drm_device *dev)
> +{
> +	return dev->mode_config.funcs->atomic_commit ? true : false;
> +}
>  
>  #endif /* DRM_ATOMIC_H_ */
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Intel-gfx] [PATCH v2 1/2] drm: Wrap the check for atomic_commit implementation
       [not found]       ` <1482395761.4759.32.camel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2016-12-22  8:52         ` Daniel Vetter
  2016-12-22  9:14           ` Pandiyan, Dhinakaran
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Vetter @ 2016-12-22  8:52 UTC (permalink / raw)
  To: Ander Conselvan De Oliveira
  Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Dhinakaran Pandiyan,
	Ben Skeggs

On Thu, Dec 22, 2016 at 10:36:01AM +0200, Ander Conselvan De Oliveira wrote:
> On Wed, 2016-12-21 at 12:12 -0800, Dhinakaran Pandiyan wrote:
> > This check is useful for drivers that do not have DRIVER_ATOMIC set but
> > have atomic modesetting internally implemented. Wrap the check into a
> > function since this is used in many places and as a bonus, the function
> > name helps to document what the check is for.
> > 
> > v2:
> > Change return type to bool (Ville)
> > Move the function drm_atomic.h (Daniel)
> > 
> > Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Ben Skeggs <bskeggs@redhat.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  drivers/gpu/drm/drm_fb_helper.c             |  6 +++---
> >  drivers/gpu/drm/nouveau/nouveau_connector.c |  5 +++--
> >  drivers/gpu/drm/nouveau/nouveau_display.c   |  6 +++---
> >  drivers/gpu/drm/nouveau/nouveau_fbcon.c     |  3 ++-
> >  include/drm/drm_atomic.h                    | 11 +++++++++++
> >  5 files changed, 22 insertions(+), 9 deletions(-)
> > 
> 
> ...
> 
> > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> > index 8cc7ca2..43db162 100644
> > --- a/include/drm/drm_atomic.h
> > +++ b/include/drm/drm_atomic.h
> > @@ -419,5 +419,16 @@ drm_atomic_crtc_needs_modeset(const struct drm_crtc_state
> > *state)
> >  	       state->connectors_changed;
> >  }
> >  
> > +/* drm_drv_uses_atomic_modeset - check if the driver implements
> 
> Shouldn't this be
> 
> /**
>  * drm_drv_uses_atomic_modeset - ...
> 
> so it is included in the generated documentation?

Yup. I'm blind this week it seems. DK, please run

$ make DOCBOOKS="" htmldocs

and make sure your new documentation does get rendered correctly and shows
up in the output.
-Daniel

> 
> Ander
> 
> 
> > + * atomic_commit()
> > + * @dev: DRM device
> > + *
> > + * This check is useful if drivers do not have DRIVER_ATOMIC set but
> > + * have atomic modesetting internally implemented.
> > + */
> > +static inline bool drm_drv_uses_atomic_modeset(struct drm_device *dev)
> > +{
> > +	return dev->mode_config.funcs->atomic_commit ? true : false;
> > +}
> >  
> >  #endif /* DRM_ATOMIC_H_ */

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/2] drm: Wrap the check for atomic_commit implementation
  2016-12-22  8:52         ` Daniel Vetter
@ 2016-12-22  9:14           ` Pandiyan, Dhinakaran
  0 siblings, 0 replies; 16+ messages in thread
From: Pandiyan, Dhinakaran @ 2016-12-22  9:14 UTC (permalink / raw)
  To: Daniel Vetter, Ander Conselvan De Oliveira
  Cc: Daniel Vetter, intel-gfx, Ben Skeggs, dri-devel, nouveau

Done, hope I got it right this time. 

-DK

-----Original Message-----
From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Thursday, December 22, 2016 12:52 AM
To: Ander Conselvan De Oliveira <conselvan2@gmail.com>
Cc: Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>; intel-gfx@lists.freedesktop.org; Daniel Vetter <daniel.vetter@ffwll.ch>; dri-devel@lists.freedesktop.org; Ben Skeggs <bskeggs@redhat.com>; nouveau@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v2 1/2] drm: Wrap the check for atomic_commit implementation

On Thu, Dec 22, 2016 at 10:36:01AM +0200, Ander Conselvan De Oliveira wrote:
> On Wed, 2016-12-21 at 12:12 -0800, Dhinakaran Pandiyan wrote:
> > This check is useful for drivers that do not have DRIVER_ATOMIC set 
> > but have atomic modesetting internally implemented. Wrap the check 
> > into a function since this is used in many places and as a bonus, 
> > the function name helps to document what the check is for.
> > 
> > v2:
> > Change return type to bool (Ville)
> > Move the function drm_atomic.h (Daniel)
> > 
> > Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Ben Skeggs <bskeggs@redhat.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  drivers/gpu/drm/drm_fb_helper.c             |  6 +++---
> >  drivers/gpu/drm/nouveau/nouveau_connector.c |  5 +++--
> >  drivers/gpu/drm/nouveau/nouveau_display.c   |  6 +++---
> >  drivers/gpu/drm/nouveau/nouveau_fbcon.c     |  3 ++-
> >  include/drm/drm_atomic.h                    | 11 +++++++++++
> >  5 files changed, 22 insertions(+), 9 deletions(-)
> > 
> 
> ...
> 
> > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h 
> > index 8cc7ca2..43db162 100644
> > --- a/include/drm/drm_atomic.h
> > +++ b/include/drm/drm_atomic.h
> > @@ -419,5 +419,16 @@ drm_atomic_crtc_needs_modeset(const struct 
> > drm_crtc_state
> > *state)
> >  	       state->connectors_changed;
> >  }
> >  
> > +/* drm_drv_uses_atomic_modeset - check if the driver implements
> 
> Shouldn't this be
> 
> /**
>  * drm_drv_uses_atomic_modeset - ...
> 
> so it is included in the generated documentation?

Yup. I'm blind this week it seems. DK, please run

$ make DOCBOOKS="" htmldocs

and make sure your new documentation does get rendered correctly and shows up in the output.
-Daniel

> 
> Ander
> 
> 
> > + * atomic_commit()
> > + * @dev: DRM device
> > + *
> > + * This check is useful if drivers do not have DRIVER_ATOMIC set 
> > +but
> > + * have atomic modesetting internally implemented.
> > + */
> > +static inline bool drm_drv_uses_atomic_modeset(struct drm_device 
> > +*dev) {
> > +	return dev->mode_config.funcs->atomic_commit ? true : false; }
> >  
> >  #endif /* DRM_ATOMIC_H_ */

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/2] drm: Get atomic property value even if DRIVER_ATOMIC is not set
  2016-12-21 20:12     ` [PATCH v2 2/2] drm: Get atomic property value even if DRIVER_ATOMIC is not set Dhinakaran Pandiyan
@ 2017-01-03 16:06       ` Alex Deucher
  0 siblings, 0 replies; 16+ messages in thread
From: Alex Deucher @ 2017-01-03 16:06 UTC (permalink / raw)
  To: Dhinakaran Pandiyan, Andrey Grodzovsky, Wentland, Harry
  Cc: Daniel Vetter, Intel Graphics Development,
	Maling list - DRI developers, nouveau

On Wed, Dec 21, 2016 at 3:12 PM, Dhinakaran Pandiyan
<dhinakaran.pandiyan@intel.com> wrote:
> i915 does not set DRIVER_ATOMIC by default yet but uses atomic_check and
> atomic_commit. drm_object_property_get_value() does not read the correct
> value of atomic properties if DRIVER_ATOMIC is not set. Checking whether
> the driver uses atomic modeset is a better check instead as the property
> values are tracked in the state structures.
>

I think we are in the same boat.  Adding Andrey and Harry.

Alex

> v2: Included header
>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/drm_mode_object.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
> index 9f17085..14543ff 100644
> --- a/drivers/gpu/drm/drm_mode_object.c
> +++ b/drivers/gpu/drm/drm_mode_object.c
> @@ -23,6 +23,7 @@
>  #include <linux/export.h>
>  #include <drm/drmP.h>
>  #include <drm/drm_mode_object.h>
> +#include <drm/drm_atomic.h>
>
>  #include "drm_crtc_internal.h"
>
> @@ -273,7 +274,7 @@ int drm_object_property_get_value(struct drm_mode_object *obj,
>          * their value in obj->properties->values[].. mostly to avoid
>          * having to deal w/ EDID and similar props in atomic paths:
>          */
> -       if (drm_core_check_feature(property->dev, DRIVER_ATOMIC) &&
> +       if (drm_drv_uses_atomic_modeset(property->dev) &&
>                         !(property->flags & DRM_MODE_PROP_IMMUTABLE))
>                 return drm_atomic_get_property(obj, property, val);
>
> --
> 2.7.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2017-01-03 16:06 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-21  1:03 [PATCH 1/2] drm: Wrap the check for atomic_commit implementation Dhinakaran Pandiyan
2016-12-21  1:03 ` [PATCH 2/2] drm: Get atomic property value even if DRIVER_ATOMIC is not set Dhinakaran Pandiyan
2016-12-21  8:53   ` Daniel Vetter
2016-12-21  1:45 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm: Wrap the check for atomic_commit implementation Patchwork
2016-12-21  8:51 ` [PATCH 1/2] " Daniel Vetter
2016-12-21 20:12   ` [PATCH v2 " Dhinakaran Pandiyan
2016-12-21 20:12     ` [PATCH v2 2/2] drm: Get atomic property value even if DRIVER_ATOMIC is not set Dhinakaran Pandiyan
2017-01-03 16:06       ` Alex Deucher
2016-12-22  7:11     ` [PATCH v2 1/2] drm: Wrap the check for atomic_commit implementation Daniel Vetter
2016-12-22  8:07     ` kbuild test robot
2016-12-22  8:36     ` [Intel-gfx] " Ander Conselvan De Oliveira
     [not found]       ` <1482395761.4759.32.camel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-12-22  8:52         ` Daniel Vetter
2016-12-22  9:14           ` Pandiyan, Dhinakaran
2016-12-21  9:23 ` [PATCH " Ville Syrjälä
2016-12-21 20:31 ` ✗ Fi.CI.BAT: failure for series starting with [v2,2/2] drm: Get atomic property value even if DRIVER_ATOMIC is not set (rev3) Patchwork
2016-12-21 22:14   ` Pandiyan, Dhinakaran

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.