drm/fb-helper: pass physical dimensions to fbdev

PATCH Superseded David Lechner 2 participants 4 messages Aug 3, 2017 – Aug 4, 2017

Subsystems (2)

DRM drivers DRM misc

lore

Share & tools

lore

David Lechner

author

2017-08-03 16:19 UTC #0 lore raw

v3 cover drm/fb-helper: pass physical dimensions to fbdev

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(-)

Signature · 1 line

2.7.4

David Lechner

author

2017-08-03 16:19 UTC #1 lore raw

PATCH v3 1/2 drm/fb-helper: add new drm_setup_crtcs_fb() function

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 <[email protected]>

---
 drivers/gpu/drm/drm_fb_helper.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

drivers/gpu/drm/drm_fb_helper.c +18 ~~-12~~

index b2a01d1..24a721a 100644
@@ -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);

Signature · 6 lines

2.7.4

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

David Lechner

author

2017-08-03 16:19 UTC #2 lore raw

PATCH v3 2/2 drm/fb-helper: pass physical dimensions to fbdev

In reply to David Lechner

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 <[email protected]>

---
 drivers/gpu/drm/drm_fb_helper.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

drivers/gpu/drm/drm_fb_helper.c +13 -2

index 24a721a..a98bf86 100644
@@ -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. */

Signature · 1 line

2.7.4

Daniel Vetter

2017-08-04 08:58 UTC #3 lore raw

Quoted 37 lines · David Lechner

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 <[email protected]>

---
 drivers/gpu/drm/drm_fb_helper.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

drivers/gpu/drm/drm_fb_helper.c +3 -2

index 24a721a..a98bf86 100644
@@ -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) {

drivers/gpu/drm/drm_fb_helper.c static void drm_setup_crtcs(struct drm_fb_helper *fb_helper, · line 2453

Quoted 10 lines

drivers/gpu/drm/drm_fb_helper.c +10 -0

+		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

Quoted 10 lines

drivers/gpu/drm/drm_fb_helper.c +0 -0

 }
 
 /* Note: Drops fb_helper->lock before returning. */

Signature · 6 lines

2.7.4

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Signature · 7 lines

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