All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] drm: get fbdev size from cmdline mode if it exists
@ 2017-01-10 11:21 Vincent Abriou
  2017-01-10 11:39 ` Daniel Vetter
  0 siblings, 1 reply; 9+ messages in thread
From: Vincent Abriou @ 2017-01-10 11:21 UTC (permalink / raw)
  To: dri-devel; +Cc: Yannick Fertre, Tomi Valkeinen, Vincent Abriou, Fabien Dessenne

In case no connector is found while creating the fbdev, gives the
possibility to specify the default fbdev size by firstly checking if the
command line is defining a preferred mode. Else go into fallback and set
1024x768 fbdev size as it was already done.

Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Vincent Abriou <vincent.abriou@st.com>
---
Patch v2:
 add a break in the connector for loop when a first cmdline mode is found

 drivers/gpu/drm/drm_fb_helper.c | 34 +++++++++++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 0ab6aaa..b38285f 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1526,6 +1526,7 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 	}
 
 	crtc_count = 0;
+
 	for (i = 0; i < fb_helper->crtc_count; i++) {
 		struct drm_display_mode *desired_mode;
 		struct drm_mode_set *mode_set;
@@ -1570,11 +1571,34 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 	}
 
 	if (crtc_count == 0 || sizes.fb_width == -1 || sizes.fb_height == -1) {
-		/* hmm everyone went away - assume VGA cable just fell out
-		   and will come back later. */
-		DRM_INFO("Cannot find any crtc or sizes - going 1024x768\n");
-		sizes.fb_width = sizes.surface_width = 1024;
-		sizes.fb_height = sizes.surface_height = 768;
+		struct drm_display_mode *mode = NULL;
+		/* hmm everyone went away - assume cable just fell out and will
+		 * come back later.
+		 * Get fb size from command line mode (if existing) else fb size
+		 * is set to 1024x768
+		 */
+		for (i = 0; i < fb_helper->connector_count; i++) {
+			struct drm_fb_helper_connector *fb_helper_conn;
+
+			fb_helper_conn = fb_helper->connector_info[i];
+			mode = drm_pick_cmdline_mode(fb_helper_conn);
+			if (mode)
+				break;
+		}
+
+		if (mode) {
+			sizes.fb_width = mode->hdisplay;
+			sizes.fb_height = mode->vdisplay;
+			DRM_INFO("Cannot find any crtc or sizes - use cmdline %dx%d\n",
+				 sizes.fb_width, sizes.fb_height);
+		} else {
+			sizes.fb_width = 1024;
+			sizes.fb_height = 768;
+			DRM_INFO("Cannot find any crtc or sizes - going 1024x768\n");
+		}
+
+		sizes.surface_width = sizes.fb_width;
+		sizes.surface_height = sizes.fb_height;
 	}
 
 	/* push down into drivers */
-- 
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] 9+ messages in thread

* Re: [PATCH v2] drm: get fbdev size from cmdline mode if it exists
  2017-01-10 11:21 [PATCH v2] drm: get fbdev size from cmdline mode if it exists Vincent Abriou
@ 2017-01-10 11:39 ` Daniel Vetter
  2017-01-10 13:33   ` Vincent ABRIOU
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Vetter @ 2017-01-10 11:39 UTC (permalink / raw)
  To: Vincent Abriou; +Cc: Yannick Fertre, Tomi Valkeinen, Fabien Dessenne, dri-devel

On Tue, Jan 10, 2017 at 12:21:09PM +0100, Vincent Abriou wrote:
> In case no connector is found while creating the fbdev, gives the
> possibility to specify the default fbdev size by firstly checking if the
> command line is defining a preferred mode. Else go into fallback and set
> 1024x768 fbdev size as it was already done.
> 
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Signed-off-by: Vincent Abriou <vincent.abriou@st.com>

btw on all this there's also the possible solution to delay setup of the
fbdev until the first connector switches to connected, and then only
allocting the fb and doing the setup. Tegra has that, and Thierry did some
patches to move that logic into the fb helpers. But there's some locking
issues that need to be fixed first, hence why those patches haven't landed
yet.

But if you never probe the right mode, this here sounds like a good idea
too.
-Daniel
> ---
> Patch v2:
>  add a break in the connector for loop when a first cmdline mode is found
> 
>  drivers/gpu/drm/drm_fb_helper.c | 34 +++++++++++++++++++++++++++++-----
>  1 file changed, 29 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 0ab6aaa..b38285f 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1526,6 +1526,7 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
>  	}
>  
>  	crtc_count = 0;
> +
>  	for (i = 0; i < fb_helper->crtc_count; i++) {
>  		struct drm_display_mode *desired_mode;
>  		struct drm_mode_set *mode_set;
> @@ -1570,11 +1571,34 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
>  	}
>  
>  	if (crtc_count == 0 || sizes.fb_width == -1 || sizes.fb_height == -1) {
> -		/* hmm everyone went away - assume VGA cable just fell out
> -		   and will come back later. */
> -		DRM_INFO("Cannot find any crtc or sizes - going 1024x768\n");
> -		sizes.fb_width = sizes.surface_width = 1024;
> -		sizes.fb_height = sizes.surface_height = 768;
> +		struct drm_display_mode *mode = NULL;
> +		/* hmm everyone went away - assume cable just fell out and will
> +		 * come back later.
> +		 * Get fb size from command line mode (if existing) else fb size
> +		 * is set to 1024x768
> +		 */
> +		for (i = 0; i < fb_helper->connector_count; i++) {
> +			struct drm_fb_helper_connector *fb_helper_conn;
> +
> +			fb_helper_conn = fb_helper->connector_info[i];
> +			mode = drm_pick_cmdline_mode(fb_helper_conn);
> +			if (mode)
> +				break;
> +		}
> +
> +		if (mode) {
> +			sizes.fb_width = mode->hdisplay;
> +			sizes.fb_height = mode->vdisplay;
> +			DRM_INFO("Cannot find any crtc or sizes - use cmdline %dx%d\n",
> +				 sizes.fb_width, sizes.fb_height);
> +		} else {
> +			sizes.fb_width = 1024;
> +			sizes.fb_height = 768;
> +			DRM_INFO("Cannot find any crtc or sizes - going 1024x768\n");
> +		}
> +
> +		sizes.surface_width = sizes.fb_width;
> +		sizes.surface_height = sizes.fb_height;
>  	}
>  
>  	/* push down into drivers */
> -- 
> 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] 9+ messages in thread

* Re: [PATCH v2] drm: get fbdev size from cmdline mode if it exists
  2017-01-10 11:39 ` Daniel Vetter
@ 2017-01-10 13:33   ` Vincent ABRIOU
  2017-01-10 20:06     ` Laurent Pinchart
  0 siblings, 1 reply; 9+ messages in thread
From: Vincent ABRIOU @ 2017-01-10 13:33 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Yannick FERTRE, Tomi Valkeinen, Fabien DESSENNE,
	dri-devel@lists.freedesktop.org



On 01/10/2017 12:39 PM, Daniel Vetter wrote:
> On Tue, Jan 10, 2017 at 12:21:09PM +0100, Vincent Abriou wrote:
>> In case no connector is found while creating the fbdev, gives the
>> possibility to specify the default fbdev size by firstly checking if the
>> command line is defining a preferred mode. Else go into fallback and set
>> 1024x768 fbdev size as it was already done.
>>
>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> Signed-off-by: Vincent Abriou <vincent.abriou@st.com>
>
> btw on all this there's also the possible solution to delay setup of the
> fbdev until the first connector switches to connected, and then only
> allocting the fb and doing the setup. Tegra has that, and Thierry did some
> patches to move that logic into the fb helpers. But there's some locking
> issues that need to be fixed first, hence why those patches haven't landed
> yet.
>
> But if you never probe the right mode, this here sounds like a good idea
> too.
> -Daniel

The early creation of fbdev is useful for userland system. If fbdev 
creation is delayed until first connector is connected, userland systems 
startup could fails (like Android that check fbdev availability at startup).

Today if no connector is connected, a default 1024x768 fbdev is created 
but it does not necessarily match the targeted CRTC size. When the 
connector is connected, fbdev is not reconfigured with the targeted CRTC 
size and it is anyway too late for the userland to take into account new 
fbdev size.
Reading the cmdline is an easy way to solve this.

Regards,
Vincent

>> ---
>> Patch v2:
>>  add a break in the connector for loop when a first cmdline mode is found
>>
>>  drivers/gpu/drm/drm_fb_helper.c | 34 +++++++++++++++++++++++++++++-----
>>  1 file changed, 29 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
>> index 0ab6aaa..b38285f 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -1526,6 +1526,7 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
>>  	}
>>
>>  	crtc_count = 0;
>> +
>>  	for (i = 0; i < fb_helper->crtc_count; i++) {
>>  		struct drm_display_mode *desired_mode;
>>  		struct drm_mode_set *mode_set;
>> @@ -1570,11 +1571,34 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
>>  	}
>>
>>  	if (crtc_count == 0 || sizes.fb_width == -1 || sizes.fb_height == -1) {
>> -		/* hmm everyone went away - assume VGA cable just fell out
>> -		   and will come back later. */
>> -		DRM_INFO("Cannot find any crtc or sizes - going 1024x768\n");
>> -		sizes.fb_width = sizes.surface_width = 1024;
>> -		sizes.fb_height = sizes.surface_height = 768;
>> +		struct drm_display_mode *mode = NULL;
>> +		/* hmm everyone went away - assume cable just fell out and will
>> +		 * come back later.
>> +		 * Get fb size from command line mode (if existing) else fb size
>> +		 * is set to 1024x768
>> +		 */
>> +		for (i = 0; i < fb_helper->connector_count; i++) {
>> +			struct drm_fb_helper_connector *fb_helper_conn;
>> +
>> +			fb_helper_conn = fb_helper->connector_info[i];
>> +			mode = drm_pick_cmdline_mode(fb_helper_conn);
>> +			if (mode)
>> +				break;
>> +		}
>> +
>> +		if (mode) {
>> +			sizes.fb_width = mode->hdisplay;
>> +			sizes.fb_height = mode->vdisplay;
>> +			DRM_INFO("Cannot find any crtc or sizes - use cmdline %dx%d\n",
>> +				 sizes.fb_width, sizes.fb_height);
>> +		} else {
>> +			sizes.fb_width = 1024;
>> +			sizes.fb_height = 768;
>> +			DRM_INFO("Cannot find any crtc or sizes - going 1024x768\n");
>> +		}
>> +
>> +		sizes.surface_width = sizes.fb_width;
>> +		sizes.surface_height = sizes.fb_height;
>>  	}
>>
>>  	/* push down into drivers */
>> --
>> 2.7.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH v2] drm: get fbdev size from cmdline mode if it exists
  2017-01-10 13:33   ` Vincent ABRIOU
@ 2017-01-10 20:06     ` Laurent Pinchart
  2017-01-11  7:48       ` Daniel Vetter
  0 siblings, 1 reply; 9+ messages in thread
From: Laurent Pinchart @ 2017-01-10 20:06 UTC (permalink / raw)
  To: dri-devel; +Cc: Yannick FERTRE, Tomi Valkeinen, Vincent ABRIOU, Fabien DESSENNE

Hi Vincent,

On Tuesday 10 Jan 2017 13:33:29 Vincent ABRIOU wrote:
> On 01/10/2017 12:39 PM, Daniel Vetter wrote:
> > On Tue, Jan 10, 2017 at 12:21:09PM +0100, Vincent Abriou wrote:
> >> In case no connector is found while creating the fbdev, gives the
> >> possibility to specify the default fbdev size by firstly checking if the
> >> command line is defining a preferred mode. Else go into fallback and set
> >> 1024x768 fbdev size as it was already done.
> >> 
> >> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> >> Signed-off-by: Vincent Abriou <vincent.abriou@st.com>
> > 
> > btw on all this there's also the possible solution to delay setup of the
> > fbdev until the first connector switches to connected, and then only
> > allocting the fb and doing the setup. Tegra has that, and Thierry did some
> > patches to move that logic into the fb helpers. But there's some locking
> > issues that need to be fixed first, hence why those patches haven't landed
> > yet.
> > 
> > But if you never probe the right mode, this here sounds like a good idea
> > too.
> 
> The early creation of fbdev is useful for userland system. If fbdev
> creation is delayed until first connector is connected, userland systems
> startup could fails (like Android that check fbdev availability at startup).
> 
> Today if no connector is connected, a default 1024x768 fbdev is created
> but it does not necessarily match the targeted CRTC size. When the
> connector is connected, fbdev is not reconfigured with the targeted CRTC
> size and it is anyway too late for the userland to take into account new
> fbdev size. Reading the cmdline is an easy way to solve this.

Isn't it another case of working around userspace issue in the kernel ? 
Shouldn't the offending userspace code be fixed ? And while at it, moved from 
fbdev to DRM/KMS ? :-) I wrote a proof-of-concept patch a few years ago to use 
DRM/KMS in the Android init process. I'm sure it doesn't apply cleanly 
anymore, but I can share it if you're interested.

-- 
Regards,

Laurent Pinchart

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

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

* Re: [PATCH v2] drm: get fbdev size from cmdline mode if it exists
  2017-01-10 20:06     ` Laurent Pinchart
@ 2017-01-11  7:48       ` Daniel Vetter
  2017-01-11  9:03         ` Vincent ABRIOU
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Vetter @ 2017-01-11  7:48 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: dri-devel, Yannick FERTRE, Tomi Valkeinen, Fabien DESSENNE,
	Vincent ABRIOU

On Tue, Jan 10, 2017 at 10:06:44PM +0200, Laurent Pinchart wrote:
> Hi Vincent,
> 
> On Tuesday 10 Jan 2017 13:33:29 Vincent ABRIOU wrote:
> > On 01/10/2017 12:39 PM, Daniel Vetter wrote:
> > > On Tue, Jan 10, 2017 at 12:21:09PM +0100, Vincent Abriou wrote:
> > >> In case no connector is found while creating the fbdev, gives the
> > >> possibility to specify the default fbdev size by firstly checking if the
> > >> command line is defining a preferred mode. Else go into fallback and set
> > >> 1024x768 fbdev size as it was already done.
> > >> 
> > >> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > >> Signed-off-by: Vincent Abriou <vincent.abriou@st.com>
> > > 
> > > btw on all this there's also the possible solution to delay setup of the
> > > fbdev until the first connector switches to connected, and then only
> > > allocting the fb and doing the setup. Tegra has that, and Thierry did some
> > > patches to move that logic into the fb helpers. But there's some locking
> > > issues that need to be fixed first, hence why those patches haven't landed
> > > yet.
> > > 
> > > But if you never probe the right mode, this here sounds like a good idea
> > > too.
> > 
> > The early creation of fbdev is useful for userland system. If fbdev
> > creation is delayed until first connector is connected, userland systems
> > startup could fails (like Android that check fbdev availability at startup).
> > 
> > Today if no connector is connected, a default 1024x768 fbdev is created
> > but it does not necessarily match the targeted CRTC size. When the
> > connector is connected, fbdev is not reconfigured with the targeted CRTC
> > size and it is anyway too late for the userland to take into account new
> > fbdev size. Reading the cmdline is an easy way to solve this.
> 
> Isn't it another case of working around userspace issue in the kernel ? 
> Shouldn't the offending userspace code be fixed ? And while at it, moved from 
> fbdev to DRM/KMS ? :-) I wrote a proof-of-concept patch a few years ago to use 
> DRM/KMS in the Android init process. I'm sure it doesn't apply cleanly 
> anymore, but I can share it if you're interested.

So android still doesn't boot without fbdev? I thought that's been fixed
...
-Daniel
-- 
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] 9+ messages in thread

* Re: [PATCH v2] drm: get fbdev size from cmdline mode if it exists
  2017-01-11  7:48       ` Daniel Vetter
@ 2017-01-11  9:03         ` Vincent ABRIOU
  2017-01-11 11:11           ` Laurent Pinchart
  0 siblings, 1 reply; 9+ messages in thread
From: Vincent ABRIOU @ 2017-01-11  9:03 UTC (permalink / raw)
  To: Daniel Vetter, Laurent Pinchart
  Cc: Yannick FERTRE, Tomi Valkeinen, Fabien DESSENNE,
	dri-devel@lists.freedesktop.org



On 01/11/2017 08:48 AM, Daniel Vetter wrote:
> On Tue, Jan 10, 2017 at 10:06:44PM +0200, Laurent Pinchart wrote:
>> Hi Vincent,
>>
>> On Tuesday 10 Jan 2017 13:33:29 Vincent ABRIOU wrote:
>>> On 01/10/2017 12:39 PM, Daniel Vetter wrote:
>>>> On Tue, Jan 10, 2017 at 12:21:09PM +0100, Vincent Abriou wrote:
>>>>> In case no connector is found while creating the fbdev, gives the
>>>>> possibility to specify the default fbdev size by firstly checking if the
>>>>> command line is defining a preferred mode. Else go into fallback and set
>>>>> 1024x768 fbdev size as it was already done.
>>>>>
>>>>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
>>>>> Signed-off-by: Vincent Abriou <vincent.abriou@st.com>
>>>>
>>>> btw on all this there's also the possible solution to delay setup of the
>>>> fbdev until the first connector switches to connected, and then only
>>>> allocting the fb and doing the setup. Tegra has that, and Thierry did some
>>>> patches to move that logic into the fb helpers. But there's some locking
>>>> issues that need to be fixed first, hence why those patches haven't landed
>>>> yet.
>>>>
>>>> But if you never probe the right mode, this here sounds like a good idea
>>>> too.
>>>
>>> The early creation of fbdev is useful for userland system. If fbdev
>>> creation is delayed until first connector is connected, userland systems
>>> startup could fails (like Android that check fbdev availability at startup).
>>>
>>> Today if no connector is connected, a default 1024x768 fbdev is created
>>> but it does not necessarily match the targeted CRTC size. When the
>>> connector is connected, fbdev is not reconfigured with the targeted CRTC
>>> size and it is anyway too late for the userland to take into account new
>>> fbdev size. Reading the cmdline is an easy way to solve this.
>>
>> Isn't it another case of working around userspace issue in the kernel ?
>> Shouldn't the offending userspace code be fixed ? And while at it, moved from
>> fbdev to DRM/KMS ? :-) I wrote a proof-of-concept patch a few years ago to use
>> DRM/KMS in the Android init process. I'm sure it doesn't apply cleanly
>> anymore, but I can share it if you're interested.

Android is one example and it still give the possibility to start with 
fbdev.
I'm not trying to fixe userspace issue (there is so many userspaces :)). 
The default case to set fbdev to 1024x768 already exist in the drm fb 
helpers. I just add a way to be flexible.

Vincent

>
> So android still doesn't boot without fbdev? I thought that's been fixed
> ...
> -Daniel
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH v2] drm: get fbdev size from cmdline mode if it exists
  2017-01-11  9:03         ` Vincent ABRIOU
@ 2017-01-11 11:11           ` Laurent Pinchart
  2017-01-11 12:31             ` Daniel Vetter
  2017-01-11 12:40             ` Vincent ABRIOU
  0 siblings, 2 replies; 9+ messages in thread
From: Laurent Pinchart @ 2017-01-11 11:11 UTC (permalink / raw)
  To: Vincent ABRIOU
  Cc: Yannick FERTRE, Tomi Valkeinen, dri-devel@lists.freedesktop.org,
	Fabien DESSENNE

Hi Vincent,

On Wednesday 11 Jan 2017 09:03:07 Vincent ABRIOU wrote:
> On 01/11/2017 08:48 AM, Daniel Vetter wrote:
> > On Tue, Jan 10, 2017 at 10:06:44PM +0200, Laurent Pinchart wrote:
> >> On Tuesday 10 Jan 2017 13:33:29 Vincent ABRIOU wrote:
> >>> On 01/10/2017 12:39 PM, Daniel Vetter wrote:
> >>>> On Tue, Jan 10, 2017 at 12:21:09PM +0100, Vincent Abriou wrote:
> >>>>> In case no connector is found while creating the fbdev, gives the
> >>>>> possibility to specify the default fbdev size by firstly checking if
> >>>>> the command line is defining a preferred mode. Else go into fallback
> >>>>> and set 1024x768 fbdev size as it was already done.
> >>>>> 
> >>>>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> >>>>> Signed-off-by: Vincent Abriou <vincent.abriou@st.com>
> >>>> 
> >>>> btw on all this there's also the possible solution to delay setup of
> >>>> the fbdev until the first connector switches to connected, and then
> >>>> only allocting the fb and doing the setup. Tegra has that, and Thierry
> >>>> did some patches to move that logic into the fb helpers. But there's
> >>>> some locking issues that need to be fixed first, hence why those
> >>>> patches haven't landed yet.
> >>>> 
> >>>> But if you never probe the right mode, this here sounds like a good
> >>>> idea too.
> >>> 
> >>> The early creation of fbdev is useful for userland system. If fbdev
> >>> creation is delayed until first connector is connected, userland systems
> >>> startup could fails (like Android that check fbdev availability at
> >>> startup).
> >>> 
> >>> Today if no connector is connected, a default 1024x768 fbdev is created
> >>> but it does not necessarily match the targeted CRTC size. When the
> >>> connector is connected, fbdev is not reconfigured with the targeted CRTC
> >>> size and it is anyway too late for the userland to take into account new
> >>> fbdev size. Reading the cmdline is an easy way to solve this.
> >> 
> >> Isn't it another case of working around userspace issue in the kernel ?
> >> Shouldn't the offending userspace code be fixed ? And while at it, moved
> >> from fbdev to DRM/KMS ? :-) I wrote a proof-of-concept patch a few years
> >> ago to use DRM/KMS in the Android init process. I'm sure it doesn't
> >> apply cleanly anymore, but I can share it if you're interested.
> 
> Android is one example and it still give the possibility to start with
> fbdev. I'm not trying to fixe userspace issue (there is so many userspaces
> :)). The default case to set fbdev to 1024x768 already exist in the drm fb
> helpers. I just add a way to be flexible.

Sure, I understand that. My point is that, instead of making life easy for 
userspace that hasn't switched to DRM/KMS yet, wouldn't it be time to push 
them a bit more ?

> > So android still doesn't boot without fbdev? I thought that's been fixed

I haven't checked the latest version to be honest.

-- 
Regards,

Laurent Pinchart

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

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

* Re: [PATCH v2] drm: get fbdev size from cmdline mode if it exists
  2017-01-11 11:11           ` Laurent Pinchart
@ 2017-01-11 12:31             ` Daniel Vetter
  2017-01-11 12:40             ` Vincent ABRIOU
  1 sibling, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2017-01-11 12:31 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: dri-devel@lists.freedesktop.org, Yannick FERTRE, Tomi Valkeinen,
	Fabien DESSENNE, Vincent ABRIOU

On Wed, Jan 11, 2017 at 01:11:45PM +0200, Laurent Pinchart wrote:
> Hi Vincent,
> 
> On Wednesday 11 Jan 2017 09:03:07 Vincent ABRIOU wrote:
> > On 01/11/2017 08:48 AM, Daniel Vetter wrote:
> > > On Tue, Jan 10, 2017 at 10:06:44PM +0200, Laurent Pinchart wrote:
> > >> On Tuesday 10 Jan 2017 13:33:29 Vincent ABRIOU wrote:
> > >>> On 01/10/2017 12:39 PM, Daniel Vetter wrote:
> > >>>> On Tue, Jan 10, 2017 at 12:21:09PM +0100, Vincent Abriou wrote:
> > >>>>> In case no connector is found while creating the fbdev, gives the
> > >>>>> possibility to specify the default fbdev size by firstly checking if
> > >>>>> the command line is defining a preferred mode. Else go into fallback
> > >>>>> and set 1024x768 fbdev size as it was already done.
> > >>>>> 
> > >>>>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > >>>>> Signed-off-by: Vincent Abriou <vincent.abriou@st.com>
> > >>>> 
> > >>>> btw on all this there's also the possible solution to delay setup of
> > >>>> the fbdev until the first connector switches to connected, and then
> > >>>> only allocting the fb and doing the setup. Tegra has that, and Thierry
> > >>>> did some patches to move that logic into the fb helpers. But there's
> > >>>> some locking issues that need to be fixed first, hence why those
> > >>>> patches haven't landed yet.
> > >>>> 
> > >>>> But if you never probe the right mode, this here sounds like a good
> > >>>> idea too.
> > >>> 
> > >>> The early creation of fbdev is useful for userland system. If fbdev
> > >>> creation is delayed until first connector is connected, userland systems
> > >>> startup could fails (like Android that check fbdev availability at
> > >>> startup).
> > >>> 
> > >>> Today if no connector is connected, a default 1024x768 fbdev is created
> > >>> but it does not necessarily match the targeted CRTC size. When the
> > >>> connector is connected, fbdev is not reconfigured with the targeted CRTC
> > >>> size and it is anyway too late for the userland to take into account new
> > >>> fbdev size. Reading the cmdline is an easy way to solve this.
> > >> 
> > >> Isn't it another case of working around userspace issue in the kernel ?
> > >> Shouldn't the offending userspace code be fixed ? And while at it, moved
> > >> from fbdev to DRM/KMS ? :-) I wrote a proof-of-concept patch a few years
> > >> ago to use DRM/KMS in the Android init process. I'm sure it doesn't
> > >> apply cleanly anymore, but I can share it if you're interested.
> > 
> > Android is one example and it still give the possibility to start with
> > fbdev. I'm not trying to fixe userspace issue (there is so many userspaces
> > :)). The default case to set fbdev to 1024x768 already exist in the drm fb
> > helpers. I just add a way to be flexible.
> 
> Sure, I understand that. My point is that, instead of making life easy for 
> userspace that hasn't switched to DRM/KMS yet, wouldn't it be time to push 
> them a bit more ?

Yeah, if the issue is just that by default android expects fbdev and gets
confused if it's not there, then I don't think that patch makes much
sense. If we need it because hw is horrible and this is the only way to
make fbcon work reasonably well, I'm all fine with that. Either way, other
people piping up with "I want this too" would help a lot. Even better when
they do a full review :-)
-Daniel
-- 
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] 9+ messages in thread

* Re: [PATCH v2] drm: get fbdev size from cmdline mode if it exists
  2017-01-11 11:11           ` Laurent Pinchart
  2017-01-11 12:31             ` Daniel Vetter
@ 2017-01-11 12:40             ` Vincent ABRIOU
  1 sibling, 0 replies; 9+ messages in thread
From: Vincent ABRIOU @ 2017-01-11 12:40 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Yannick FERTRE, Tomi Valkeinen, dri-devel@lists.freedesktop.org,
	Fabien DESSENNE



On 01/11/2017 12:11 PM, Laurent Pinchart wrote:
> Hi Vincent,
>
> On Wednesday 11 Jan 2017 09:03:07 Vincent ABRIOU wrote:
>> On 01/11/2017 08:48 AM, Daniel Vetter wrote:
>>> On Tue, Jan 10, 2017 at 10:06:44PM +0200, Laurent Pinchart wrote:
>>>> On Tuesday 10 Jan 2017 13:33:29 Vincent ABRIOU wrote:
>>>>> On 01/10/2017 12:39 PM, Daniel Vetter wrote:
>>>>>> On Tue, Jan 10, 2017 at 12:21:09PM +0100, Vincent Abriou wrote:
>>>>>>> In case no connector is found while creating the fbdev, gives the
>>>>>>> possibility to specify the default fbdev size by firstly checking if
>>>>>>> the command line is defining a preferred mode. Else go into fallback
>>>>>>> and set 1024x768 fbdev size as it was already done.
>>>>>>>
>>>>>>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
>>>>>>> Signed-off-by: Vincent Abriou <vincent.abriou@st.com>
>>>>>>
>>>>>> btw on all this there's also the possible solution to delay setup of
>>>>>> the fbdev until the first connector switches to connected, and then
>>>>>> only allocting the fb and doing the setup. Tegra has that, and Thierry
>>>>>> did some patches to move that logic into the fb helpers. But there's
>>>>>> some locking issues that need to be fixed first, hence why those
>>>>>> patches haven't landed yet.
>>>>>>
>>>>>> But if you never probe the right mode, this here sounds like a good
>>>>>> idea too.
>>>>>
>>>>> The early creation of fbdev is useful for userland system. If fbdev
>>>>> creation is delayed until first connector is connected, userland systems
>>>>> startup could fails (like Android that check fbdev availability at
>>>>> startup).
>>>>>
>>>>> Today if no connector is connected, a default 1024x768 fbdev is created
>>>>> but it does not necessarily match the targeted CRTC size. When the
>>>>> connector is connected, fbdev is not reconfigured with the targeted CRTC
>>>>> size and it is anyway too late for the userland to take into account new
>>>>> fbdev size. Reading the cmdline is an easy way to solve this.
>>>>
>>>> Isn't it another case of working around userspace issue in the kernel ?
>>>> Shouldn't the offending userspace code be fixed ? And while at it, moved
>>>> from fbdev to DRM/KMS ? :-) I wrote a proof-of-concept patch a few years
>>>> ago to use DRM/KMS in the Android init process. I'm sure it doesn't
>>>> apply cleanly anymore, but I can share it if you're interested.
>>
>> Android is one example and it still give the possibility to start with
>> fbdev. I'm not trying to fixe userspace issue (there is so many userspaces
>> :)). The default case to set fbdev to 1024x768 already exist in the drm fb
>> helpers. I just add a way to be flexible.
>
> Sure, I understand that. My point is that, instead of making life easy for
> userspace that hasn't switched to DRM/KMS yet, wouldn't it be time to push
> them a bit more ?
>
>>> So android still doesn't boot without fbdev? I thought that's been fixed
>

Android support fbdev and drm/kms.

> I haven't checked the latest version to be honest.
>

The last Android version still support fbdev.

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

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

end of thread, other threads:[~2017-01-11 12:40 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-10 11:21 [PATCH v2] drm: get fbdev size from cmdline mode if it exists Vincent Abriou
2017-01-10 11:39 ` Daniel Vetter
2017-01-10 13:33   ` Vincent ABRIOU
2017-01-10 20:06     ` Laurent Pinchart
2017-01-11  7:48       ` Daniel Vetter
2017-01-11  9:03         ` Vincent ABRIOU
2017-01-11 11:11           ` Laurent Pinchart
2017-01-11 12:31             ` Daniel Vetter
2017-01-11 12:40             ` Vincent ABRIOU

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.