All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/2] drm/fb-helper: pass physical dimensions to fbdev
@ 2017-08-03 16:19 David Lechner
  2017-08-03 16:19   ` David Lechner
  2017-08-03 16:19 ` [PATCH v3 2/2] drm/fb-helper: pass physical dimensions to fbdev David Lechner
  0 siblings, 2 replies; 6+ messages in thread
From: David Lechner @ 2017-08-03 16:19 UTC (permalink / raw)
  To: dri-devel
  Cc: David Lechner, Daniel Vetter, Jani Nikula, Sean Paul,
	David Airlie, linux-kernel

v1 changes (from RFC):
* Use loop to get info from first connected connector instead of just the
  first connector.

v2 changes:
* Update width in height in drm_setup_crtcs() also to handle hotplug events.

v3 changes:
* Add new patch to handle post-fb allocation crcts setup.
* Use new drm_setup_crtcs_fb() function from new patch to avoid duplication
  of code.

David Lechner (2):
  drm/fb-helper: add new drm_setup_crtcs_fb() function
  drm/fb-helper: pass physical dimensions to fbdev

 drivers/gpu/drm/drm_fb_helper.c | 45 ++++++++++++++++++++++++++++-------------
 1 file changed, 31 insertions(+), 14 deletions(-)

-- 
2.7.4

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

* [PATCH v3 1/2] drm/fb-helper: add new drm_setup_crtcs_fb() function
  2017-08-03 16:19 [PATCH v3 0/2] drm/fb-helper: pass physical dimensions to fbdev David Lechner
@ 2017-08-03 16:19   ` David Lechner
  2017-08-03 16:19 ` [PATCH v3 2/2] drm/fb-helper: pass physical dimensions to fbdev David Lechner
  1 sibling, 0 replies; 6+ messages in thread
From: David Lechner @ 2017-08-03 16:19 UTC (permalink / raw)
  To: dri-devel
  Cc: David Lechner, Daniel Vetter, Jani Nikula, Sean Paul,
	David Airlie, linux-kernel

This adds a new drm_setup_crtcs_fb() function to handle the parts of
drm_setup_crtcs() that touch fb_helper->fb and fb_helper->fbdev. When
drm_setup_crtcs() is called during initialization, these fields are NULL
because they have not been allocated yet.

There is currently a hack at the end of drm_fb_helper_single_fb_probe()
that sets fb_helper->fb, so it is moved to the new drm_setup_crtcs_fb()
function.

This is also done in preparation for addition setup that requires access
to fb_helper->fbdev.

Signed-off-by: David Lechner <david@lechnology.com>
---
 drivers/gpu/drm/drm_fb_helper.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index b2a01d1..24a721a 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1821,17 +1821,6 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 	if (ret < 0)
 		return ret;
 
-	/*
-	 * Set the fb pointer - usually drm_setup_crtcs does this for hotplug
-	 * events, but at init time drm_setup_crtcs needs to be called before
-	 * the fb is allocated (since we need to figure out the desired size of
-	 * the fb before we can allocate it ...). Hence we need to fix things up
-	 * here again.
-	 */
-	for (i = 0; i < fb_helper->crtc_count; i++)
-		if (fb_helper->crtc_info[i].mode_set.num_connectors)
-			fb_helper->crtc_info[i].mode_set.fb = fb_helper->fb;
-
 	return 0;
 }
 
@@ -2426,7 +2415,6 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
 							   fb_crtc->desired_mode);
 			drm_connector_get(connector);
 			modeset->connectors[modeset->num_connectors++] = connector;
-			modeset->fb = fb_helper->fb;
 			modeset->x = offset->x;
 			modeset->y = offset->y;
 		}
@@ -2438,6 +2426,22 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
 	kfree(enabled);
 }
 
+/*
+ * This is a continuation of drm_setup_crtcs() that sets up anything related
+ * to the framebuffer. During initialization, drm_setup_crtcs() is called before
+ * the framebuffer has been allocated (fb_helper->fb and fb_helper->fbdev).
+ * So, any setup that touches those fields needs to be done here instead of in
+ * drm_setup_crtcs().
+ */
+static void drm_setup_crtcs_fb(struct drm_fb_helper *fb_helper)
+{
+	int i;
+
+	for (i = 0; i < fb_helper->crtc_count; i++)
+		if (fb_helper->crtc_info[i].mode_set.num_connectors)
+			fb_helper->crtc_info[i].mode_set.fb = fb_helper->fb;
+}
+
 /* Note: Drops fb_helper->lock before returning. */
 static int
 __drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper,
@@ -2463,6 +2467,7 @@ __drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper,
 
 		return ret;
 	}
+	drm_setup_crtcs_fb(fb_helper);
 
 	fb_helper->deferred_setup = false;
 
@@ -2591,6 +2596,7 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
 	DRM_DEBUG_KMS("\n");
 
 	drm_setup_crtcs(fb_helper, fb_helper->fb->width, fb_helper->fb->height);
+	drm_setup_crtcs_fb(fb_helper);
 	mutex_unlock(&fb_helper->lock);
 
 	drm_fb_helper_set_par(fb_helper->fbdev);
-- 
2.7.4

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

* [PATCH v3 1/2] drm/fb-helper: add new drm_setup_crtcs_fb() function
@ 2017-08-03 16:19   ` David Lechner
  0 siblings, 0 replies; 6+ messages in thread
From: David Lechner @ 2017-08-03 16:19 UTC (permalink / raw)
  To: dri-devel; +Cc: David Lechner, linux-kernel, Daniel Vetter

This adds a new drm_setup_crtcs_fb() function to handle the parts of
drm_setup_crtcs() that touch fb_helper->fb and fb_helper->fbdev. When
drm_setup_crtcs() is called during initialization, these fields are NULL
because they have not been allocated yet.

There is currently a hack at the end of drm_fb_helper_single_fb_probe()
that sets fb_helper->fb, so it is moved to the new drm_setup_crtcs_fb()
function.

This is also done in preparation for addition setup that requires access
to fb_helper->fbdev.

Signed-off-by: David Lechner <david@lechnology.com>
---
 drivers/gpu/drm/drm_fb_helper.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index b2a01d1..24a721a 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1821,17 +1821,6 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 	if (ret < 0)
 		return ret;
 
-	/*
-	 * Set the fb pointer - usually drm_setup_crtcs does this for hotplug
-	 * events, but at init time drm_setup_crtcs needs to be called before
-	 * the fb is allocated (since we need to figure out the desired size of
-	 * the fb before we can allocate it ...). Hence we need to fix things up
-	 * here again.
-	 */
-	for (i = 0; i < fb_helper->crtc_count; i++)
-		if (fb_helper->crtc_info[i].mode_set.num_connectors)
-			fb_helper->crtc_info[i].mode_set.fb = fb_helper->fb;
-
 	return 0;
 }
 
@@ -2426,7 +2415,6 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
 							   fb_crtc->desired_mode);
 			drm_connector_get(connector);
 			modeset->connectors[modeset->num_connectors++] = connector;
-			modeset->fb = fb_helper->fb;
 			modeset->x = offset->x;
 			modeset->y = offset->y;
 		}
@@ -2438,6 +2426,22 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
 	kfree(enabled);
 }
 
+/*
+ * This is a continuation of drm_setup_crtcs() that sets up anything related
+ * to the framebuffer. During initialization, drm_setup_crtcs() is called before
+ * the framebuffer has been allocated (fb_helper->fb and fb_helper->fbdev).
+ * So, any setup that touches those fields needs to be done here instead of in
+ * drm_setup_crtcs().
+ */
+static void drm_setup_crtcs_fb(struct drm_fb_helper *fb_helper)
+{
+	int i;
+
+	for (i = 0; i < fb_helper->crtc_count; i++)
+		if (fb_helper->crtc_info[i].mode_set.num_connectors)
+			fb_helper->crtc_info[i].mode_set.fb = fb_helper->fb;
+}
+
 /* Note: Drops fb_helper->lock before returning. */
 static int
 __drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper,
@@ -2463,6 +2467,7 @@ __drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper,
 
 		return ret;
 	}
+	drm_setup_crtcs_fb(fb_helper);
 
 	fb_helper->deferred_setup = false;
 
@@ -2591,6 +2596,7 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
 	DRM_DEBUG_KMS("\n");
 
 	drm_setup_crtcs(fb_helper, fb_helper->fb->width, fb_helper->fb->height);
+	drm_setup_crtcs_fb(fb_helper);
 	mutex_unlock(&fb_helper->lock);
 
 	drm_fb_helper_set_par(fb_helper->fbdev);
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH v3 2/2] drm/fb-helper: pass physical dimensions to fbdev
  2017-08-03 16:19 [PATCH v3 0/2] drm/fb-helper: pass physical dimensions to fbdev David Lechner
  2017-08-03 16:19   ` David Lechner
@ 2017-08-03 16:19 ` David Lechner
  2017-08-04  8:58     ` Daniel Vetter
  1 sibling, 1 reply; 6+ messages in thread
From: David Lechner @ 2017-08-03 16:19 UTC (permalink / raw)
  To: dri-devel
  Cc: David Lechner, Daniel Vetter, Jani Nikula, Sean Paul,
	David Airlie, linux-kernel

The fbdev subsystem has a place for physical dimensions (width and height
in mm) that is readable by userspace. Since DRM also knows these
dimensions, pass this information to the fbdev device.

This has to be done in drm_setup_crtcs_fb() instead of drm_setup_crtcs()
because fb_helper->fbdev may be NULL in drm_setup_crtcs().

Signed-off-by: David Lechner <david@lechnology.com>
---
 drivers/gpu/drm/drm_fb_helper.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 24a721a..a98bf86 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1882,8 +1882,6 @@ void drm_fb_helper_fill_var(struct fb_info *info, struct drm_fb_helper *fb_helpe
 	info->var.xoffset = 0;
 	info->var.yoffset = 0;
 	info->var.activate = FB_ACTIVATE_NOW;
-	info->var.height = -1;
-	info->var.width = -1;
 
 	switch (fb->format->depth) {
 	case 8:
@@ -2435,11 +2433,24 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
  */
 static void drm_setup_crtcs_fb(struct drm_fb_helper *fb_helper)
 {
+	struct fb_info *info = fb_helper->fbdev;
 	int i;
 
 	for (i = 0; i < fb_helper->crtc_count; i++)
 		if (fb_helper->crtc_info[i].mode_set.num_connectors)
 			fb_helper->crtc_info[i].mode_set.fb = fb_helper->fb;
+
+	drm_fb_helper_for_each_connector(fb_helper, i) {
+		struct drm_connector *connector =
+					fb_helper->connector_info[i]->connector;
+
+		/* use first connected connector for the physical dimensions */
+		if (connector->status == connector_status_connected) {
+			info->var.height = connector->display_info.width_mm;
+			info->var.width = connector->display_info.height_mm;
+			break;
+		}
+	}
 }
 
 /* Note: Drops fb_helper->lock before returning. */
-- 
2.7.4

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

* Re: [PATCH v3 2/2] drm/fb-helper: pass physical dimensions to fbdev
  2017-08-03 16:19 ` [PATCH v3 2/2] drm/fb-helper: pass physical dimensions to fbdev David Lechner
@ 2017-08-04  8:58     ` Daniel Vetter
  0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2017-08-04  8:58 UTC (permalink / raw)
  To: David Lechner; +Cc: dri-devel, linux-kernel, Daniel Vetter

On Thu, Aug 03, 2017 at 11:19:09AM -0500, David Lechner wrote:
> The fbdev subsystem has a place for physical dimensions (width and height
> in mm) that is readable by userspace. Since DRM also knows these
> dimensions, pass this information to the fbdev device.
> 
> This has to be done in drm_setup_crtcs_fb() instead of drm_setup_crtcs()
> because fb_helper->fbdev may be NULL in drm_setup_crtcs().
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 24a721a..a98bf86 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1882,8 +1882,6 @@ void drm_fb_helper_fill_var(struct fb_info *info, struct drm_fb_helper *fb_helpe
>  	info->var.xoffset = 0;
>  	info->var.yoffset = 0;
>  	info->var.activate = FB_ACTIVATE_NOW;
> -	info->var.height = -1;
> -	info->var.width = -1;
>  
>  	switch (fb->format->depth) {
>  	case 8:
> @@ -2435,11 +2433,24 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
>   */
>  static void drm_setup_crtcs_fb(struct drm_fb_helper *fb_helper)
>  {
> +	struct fb_info *info = fb_helper->fbdev;
>  	int i;
>  
>  	for (i = 0; i < fb_helper->crtc_count; i++)
>  		if (fb_helper->crtc_info[i].mode_set.num_connectors)
>  			fb_helper->crtc_info[i].mode_set.fb = fb_helper->fb;
> +
> +	drm_fb_helper_for_each_connector(fb_helper, i) {
> +		struct drm_connector *connector =
> +					fb_helper->connector_info[i]->connector;
> +
> +		/* use first connected connector for the physical dimensions */
> +		if (connector->status == connector_status_connected) {
> +			info->var.height = connector->display_info.width_mm;
> +			info->var.width = connector->display_info.height_mm;
> +			break;
> +		}
> +	}

Ok, small trapdoor I didn't highlight: Accessing the connector prope state
(i.e. ->status and ->display_info) requires that you hold
dev->mode_config.mutex. See how it's done in drm_setup_crtcs, just wrap it
around this loop here.

fbdev state itself is protected by fb_helper->lock, so that part is all
good.

I've applied patch 1 already, but can you pls respin that one with the
locking fixed up?

Thanks, Daniel
>  }
>  
>  /* Note: Drops fb_helper->lock before returning. */
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

* Re: [PATCH v3 2/2] drm/fb-helper: pass physical dimensions to fbdev
@ 2017-08-04  8:58     ` Daniel Vetter
  0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2017-08-04  8:58 UTC (permalink / raw)
  To: David Lechner; +Cc: Daniel Vetter, linux-kernel, dri-devel

On Thu, Aug 03, 2017 at 11:19:09AM -0500, David Lechner wrote:
> The fbdev subsystem has a place for physical dimensions (width and height
> in mm) that is readable by userspace. Since DRM also knows these
> dimensions, pass this information to the fbdev device.
> 
> This has to be done in drm_setup_crtcs_fb() instead of drm_setup_crtcs()
> because fb_helper->fbdev may be NULL in drm_setup_crtcs().
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 24a721a..a98bf86 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1882,8 +1882,6 @@ void drm_fb_helper_fill_var(struct fb_info *info, struct drm_fb_helper *fb_helpe
>  	info->var.xoffset = 0;
>  	info->var.yoffset = 0;
>  	info->var.activate = FB_ACTIVATE_NOW;
> -	info->var.height = -1;
> -	info->var.width = -1;
>  
>  	switch (fb->format->depth) {
>  	case 8:
> @@ -2435,11 +2433,24 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
>   */
>  static void drm_setup_crtcs_fb(struct drm_fb_helper *fb_helper)
>  {
> +	struct fb_info *info = fb_helper->fbdev;
>  	int i;
>  
>  	for (i = 0; i < fb_helper->crtc_count; i++)
>  		if (fb_helper->crtc_info[i].mode_set.num_connectors)
>  			fb_helper->crtc_info[i].mode_set.fb = fb_helper->fb;
> +
> +	drm_fb_helper_for_each_connector(fb_helper, i) {
> +		struct drm_connector *connector =
> +					fb_helper->connector_info[i]->connector;
> +
> +		/* use first connected connector for the physical dimensions */
> +		if (connector->status == connector_status_connected) {
> +			info->var.height = connector->display_info.width_mm;
> +			info->var.width = connector->display_info.height_mm;
> +			break;
> +		}
> +	}

Ok, small trapdoor I didn't highlight: Accessing the connector prope state
(i.e. ->status and ->display_info) requires that you hold
dev->mode_config.mutex. See how it's done in drm_setup_crtcs, just wrap it
around this loop here.

fbdev state itself is protected by fb_helper->lock, so that part is all
good.

I've applied patch 1 already, but can you pls respin that one with the
locking fixed up?

Thanks, Daniel
>  }
>  
>  /* Note: Drops fb_helper->lock before returning. */
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

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

end of thread, other threads:[~2017-08-04  8:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-03 16:19 [PATCH v3 0/2] drm/fb-helper: pass physical dimensions to fbdev David Lechner
2017-08-03 16:19 ` [PATCH v3 1/2] drm/fb-helper: add new drm_setup_crtcs_fb() function David Lechner
2017-08-03 16:19   ` David Lechner
2017-08-03 16:19 ` [PATCH v3 2/2] drm/fb-helper: pass physical dimensions to fbdev David Lechner
2017-08-04  8:58   ` Daniel Vetter
2017-08-04  8:58     ` Daniel Vetter

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.