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
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(-)
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
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(-)
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
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 <[email protected]>
> ---
> 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
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch