diff mbox

[v2] drm: get fbdev size from cmdline mode if it exists

Message ID 1484047269-18013-1-git-send-email-vincent.abriou@st.com (mailing list archive)
State New, archived
Headers show

Commit Message

Vincent Abriou Jan. 10, 2017, 11:21 a.m. UTC
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(-)

Comments

Daniel Vetter Jan. 10, 2017, 11:39 a.m. UTC | #1
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
Vincent Abriou Jan. 10, 2017, 1:33 p.m. UTC | #2
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
>
Laurent Pinchart Jan. 10, 2017, 8:06 p.m. UTC | #3
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.
Daniel Vetter Jan. 11, 2017, 7:48 a.m. UTC | #4
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
Vincent Abriou Jan. 11, 2017, 9:03 a.m. UTC | #5
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
>
Laurent Pinchart Jan. 11, 2017, 11:11 a.m. UTC | #6
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.
Daniel Vetter Jan. 11, 2017, 12:31 p.m. UTC | #7
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
Vincent Abriou Jan. 11, 2017, 12:40 p.m. UTC | #8
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
diff mbox

Patch

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 */