[PATCH v5] drm/fb-helper: pass physical dimensions to fbdev

[PATCH v5] drm/fb-helper: pass physical dimensions to fbdev

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

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.

v4 changes:
* Drop patch "drm/fb-helper: add new drm_setup_crtcs_fb() function" since it
  has been applied already.
* Add mutex lock.

v5 changes:
* Fix height = width mismatch.

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

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 4447a28..24787d6 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,26 @@ 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;
+
+	mutex_lock(&fb_helper->dev->mode_config.mutex);
+	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.width = connector->display_info.width_mm;
+			info->var.height = connector->display_info.height_mm;
+			break;
+		}
+	}
+	mutex_unlock(&fb_helper->dev->mode_config.mutex);
 }
 
 /* Note: Drops fb_helper->lock before returning. */
-- 
2.7.4

Re: [PATCH v5] drm/fb-helper: pass physical dimensions to fbdev

[Laurent Pinchart]

Hi David,

Thank you for the patch.

On Friday 04 Aug 2017 11:25:24 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.

I'm curious, what's your use case for this ? What userspace software do you 
use that is still based on fbdev and needs screen dimensions ?

> 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().
> ---
> 
> 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.
> 
> v4 changes:
> * Drop patch "drm/fb-helper: add new drm_setup_crtcs_fb() function" since it
> has been applied already.
> * Add mutex lock.
> 
> v5 changes:
> * Fix height = width mismatch.
> 
>  drivers/gpu/drm/drm_fb_helper.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c
> b/drivers/gpu/drm/drm_fb_helper.c index 4447a28..24787d6 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,26 @@ 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;
> +
> +	mutex_lock(&fb_helper->dev->mode_config.mutex);
> +	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.width = connector->display_info.width_mm;
> +			info->var.height = connector-
>display_info.height_mm;
> +			break;
> +		}
> +	}
> +	mutex_unlock(&fb_helper->dev->mode_config.mutex);
>  }
> 
>  /* Note: Drops fb_helper->lock before returning. */

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5] drm/fb-helper: pass physical dimensions to fbdev

[David Lechner]

On 08/04/2017 04:53 PM, Laurent Pinchart wrote:
> Hi David,
> 
> Thank you for the patch.
> 
> On Friday 04 Aug 2017 11:25:24 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.
> 
> I'm curious, what's your use case for this ? What userspace software do you
> use that is still based on fbdev and needs screen dimensions ?


I have dug up an old graphics library called GRX[1] and modernized 
it[2]. So, I am using the screen dimensions to make it DPI-aware so that 
the font on my 2.4" screen looks the same size as my 2.8" screen when 
both are 320x240 pixels. Until this year, all of the displays I have 
only have fbdev drivers.

[1]: http://grx.gnu.de
[2]: https://github.com/ev3dev/grx

Re: [PATCH v5] drm/fb-helper: pass physical dimensions to fbdev

[Daniel Vetter]

On Fri, Aug 04, 2017 at 11:25:24AM -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().

You'r sob line is missing here.
-Daniel

> ---
> 
> 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.
> 
> v4 changes:
> * Drop patch "drm/fb-helper: add new drm_setup_crtcs_fb() function" since it
>   has been applied already.
> * Add mutex lock.
> 
> v5 changes:
> * Fix height = width mismatch.
> 
>  drivers/gpu/drm/drm_fb_helper.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 4447a28..24787d6 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,26 @@ 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;
> +
> +	mutex_lock(&fb_helper->dev->mode_config.mutex);
> +	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.width = connector->display_info.width_mm;
> +			info->var.height = connector->display_info.height_mm;
> +			break;
> +		}
> +	}
> +	mutex_unlock(&fb_helper->dev->mode_config.mutex);
>  }
>  
>  /* 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

Re: [PATCH v5] drm/fb-helper: pass physical dimensions to fbdev

[David Lechner]

On 08/04/2017 11:25 AM, 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>

Re: [PATCH v5] drm/fb-helper: pass physical dimensions to fbdev

[Daniel Vetter]

On Mon, Aug 07, 2017 at 08:40:35AM -0500, David Lechner wrote:
> On 08/04/2017 11:25 AM, 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>

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