Re: [PATCH 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D.

[Daniel Vetter]

On Mon, Apr 24, 2017 at 10:26:45AM -0400, Alex Deucher wrote:
> On Fri, Apr 21, 2017 at 6:53 PM, Eric Anholt  wrote:
> > Daniel Vetter  writes:
> >
> >> On Wed, Apr 19, 2017 at 7:55 PM, Eric Anholt  wrote:
> >>> Daniel Vetter  writes:
>  On Tue, Apr 18, 2017 at 9:11 PM, Eric Anholt  wrote:
> > The FBDEV initialization would throw an error in dmesg, when we just
> > want to silently not initialize fbdev on a V3D-only VC4 instance.
> >
> > Signed-off-by: Eric Anholt 
> 
>  Hm, this shouldn't be an error really, you might want to hotplug more
>  connectors later on. What exactly complains?
> >>>
> >>> drm_fb_helper_init() throws an error if the passed in connector count is
> >>> 0, so drm_fb_cma_helper() printks an error.
> >>
> >> Oh, _that_ thing. The error in there is correct, but (almost) everyone
> >> gets this parameter wrong. This isn't the max number of connectors the
> >> fb helper will light up, but just the max number of connectors _per_
> >> crtc when driving in hw clone mode. There's two problems with that:
> >> - fb helpers don't support hw clone mode, we select 1:1 crtcs for each
> >> active connector
> >> - I mentioned that everyone gets this wrong?
> >>
> >> If you're moderately bored it'd be great to nuke the max_connector
> >> argument from drm_fb_helper_init, and hard-code it to 1 (with a big
> >> comment explaining that this needs to be changed, probably with
> >> dynamic reallocation, once someone gets around to implementing hw
> >> clone mode).
> >>
> >> If you're less bored, just hardcode this to 1 in vc4 and done. Plus a
> >> TODO.rst entry would be great in that case.
> >
> > If I'm driving a GPU with no display subsystem at all, it seems like I
> > shouldn't initialize fbdev for it, right?
> 
> That's what we do for radeon/amdgpu on hw without display blocks.

Yeah I got slightly distracted with the error :-)

Still might make sense to put this logic into the helpers, if there's no
crtc then don't bother initializing the fbcon. But imo the better cleanup
is getting rid of the connectors parameter, since most drivers get it
wrong, then respin the vc4 patch to explain what it's really doing (it's
not about the error really, it's about not registering and fbdev that's
not doing anything).
-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

Re: [PATCH 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D.

[Alex Deucher]

On Fri, Apr 21, 2017 at 6:53 PM, Eric Anholt  wrote:
> Daniel Vetter  writes:
>
>> On Wed, Apr 19, 2017 at 7:55 PM, Eric Anholt  wrote:
>>> Daniel Vetter  writes:
 On Tue, Apr 18, 2017 at 9:11 PM, Eric Anholt  wrote:
> The FBDEV initialization would throw an error in dmesg, when we just
> want to silently not initialize fbdev on a V3D-only VC4 instance.
>
> Signed-off-by: Eric Anholt 

 Hm, this shouldn't be an error really, you might want to hotplug more
 connectors later on. What exactly complains?
>>>
>>> drm_fb_helper_init() throws an error if the passed in connector count is
>>> 0, so drm_fb_cma_helper() printks an error.
>>
>> Oh, _that_ thing. The error in there is correct, but (almost) everyone
>> gets this parameter wrong. This isn't the max number of connectors the
>> fb helper will light up, but just the max number of connectors _per_
>> crtc when driving in hw clone mode. There's two problems with that:
>> - fb helpers don't support hw clone mode, we select 1:1 crtcs for each
>> active connector
>> - I mentioned that everyone gets this wrong?
>>
>> If you're moderately bored it'd be great to nuke the max_connector
>> argument from drm_fb_helper_init, and hard-code it to 1 (with a big
>> comment explaining that this needs to be changed, probably with
>> dynamic reallocation, once someone gets around to implementing hw
>> clone mode).
>>
>> If you're less bored, just hardcode this to 1 in vc4 and done. Plus a
>> TODO.rst entry would be great in that case.
>
> If I'm driving a GPU with no display subsystem at all, it seems like I
> shouldn't initialize fbdev for it, right?

That's what we do for radeon/amdgpu on hw without display blocks.

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

Re: [PATCH 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D.

[Eric Anholt]

Daniel Vetter  writes:

> On Wed, Apr 19, 2017 at 7:55 PM, Eric Anholt  wrote:
>> Daniel Vetter  writes:
>>> On Tue, Apr 18, 2017 at 9:11 PM, Eric Anholt  wrote:
 The FBDEV initialization would throw an error in dmesg, when we just
 want to silently not initialize fbdev on a V3D-only VC4 instance.

 Signed-off-by: Eric Anholt 
>>>
>>> Hm, this shouldn't be an error really, you might want to hotplug more
>>> connectors later on. What exactly complains?
>>
>> drm_fb_helper_init() throws an error if the passed in connector count is
>> 0, so drm_fb_cma_helper() printks an error.
>
> Oh, _that_ thing. The error in there is correct, but (almost) everyone
> gets this parameter wrong. This isn't the max number of connectors the
> fb helper will light up, but just the max number of connectors _per_
> crtc when driving in hw clone mode. There's two problems with that:
> - fb helpers don't support hw clone mode, we select 1:1 crtcs for each
> active connector
> - I mentioned that everyone gets this wrong?
>
> If you're moderately bored it'd be great to nuke the max_connector
> argument from drm_fb_helper_init, and hard-code it to 1 (with a big
> comment explaining that this needs to be changed, probably with
> dynamic reallocation, once someone gets around to implementing hw
> clone mode).
>
> If you're less bored, just hardcode this to 1 in vc4 and done. Plus a
> TODO.rst entry would be great in that case.

If I'm driving a GPU with no display subsystem at all, it seems like I
shouldn't initialize fbdev for it, right?


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D.

[Daniel Vetter]

On Wed, Apr 19, 2017 at 7:55 PM, Eric Anholt  wrote:
> Daniel Vetter  writes:
>> On Tue, Apr 18, 2017 at 9:11 PM, Eric Anholt  wrote:
>>> The FBDEV initialization would throw an error in dmesg, when we just
>>> want to silently not initialize fbdev on a V3D-only VC4 instance.
>>>
>>> Signed-off-by: Eric Anholt 
>>
>> Hm, this shouldn't be an error really, you might want to hotplug more
>> connectors later on. What exactly complains?
>
> drm_fb_helper_init() throws an error if the passed in connector count is
> 0, so drm_fb_cma_helper() printks an error.

Oh, _that_ thing. The error in there is correct, but (almost) everyone
gets this parameter wrong. This isn't the max number of connectors the
fb helper will light up, but just the max number of connectors _per_
crtc when driving in hw clone mode. There's two problems with that:
- fb helpers don't support hw clone mode, we select 1:1 crtcs for each
active connector
- I mentioned that everyone gets this wrong?

If you're moderately bored it'd be great to nuke the max_connector
argument from drm_fb_helper_init, and hard-code it to 1 (with a big
comment explaining that this needs to be changed, probably with
dynamic reallocation, once someone gets around to implementing hw
clone mode).

If you're less bored, just hardcode this to 1 in vc4 and done. Plus a
TODO.rst entry would be great in that case.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D.

[Eric Anholt]

Daniel Vetter  writes:

> On Tue, Apr 18, 2017 at 9:11 PM, Eric Anholt  wrote:
>> The FBDEV initialization would throw an error in dmesg, when we just
>> want to silently not initialize fbdev on a V3D-only VC4 instance.
>>
>> Signed-off-by: Eric Anholt 
>
> Hm, this shouldn't be an error really, you might want to hotplug more
> connectors later on. What exactly complains?

drm_fb_helper_init() throws an error if the passed in connector count is
0, so drm_fb_cma_helper() printks an error.


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D.

[Daniel Vetter]

On Tue, Apr 18, 2017 at 9:11 PM, Eric Anholt  wrote:
> The FBDEV initialization would throw an error in dmesg, when we just
> want to silently not initialize fbdev on a V3D-only VC4 instance.
>
> Signed-off-by: Eric Anholt 

Hm, this shouldn't be an error really, you might want to hotplug more
connectors later on. What exactly complains?
-Daniel

> ---
>  drivers/gpu/drm/vc4/vc4_kms.c | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
> index ad7925a9e0ea..237a504f11f0 100644
> --- a/drivers/gpu/drm/vc4/vc4_kms.c
> +++ b/drivers/gpu/drm/vc4/vc4_kms.c
> @@ -230,10 +230,12 @@ int vc4_kms_load(struct drm_device *dev)
>
> drm_mode_config_reset(dev);
>
> -   vc4->fbdev = drm_fbdev_cma_init(dev, 32,
> -   dev->mode_config.num_connector);
> -   if (IS_ERR(vc4->fbdev))
> -   vc4->fbdev = NULL;
> +   if (dev->mode_config.num_connector) {
> +   vc4->fbdev = drm_fbdev_cma_init(dev, 32,
> +   
> dev->mode_config.num_connector);
> +   if (IS_ERR(vc4->fbdev))
> +   vc4->fbdev = NULL;
> +   }
>
> drm_kms_helper_poll_init(dev);
>
> --
> 2.11.0
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D.

[Eric Anholt]

The FBDEV initialization would throw an error in dmesg, when we just
want to silently not initialize fbdev on a V3D-only VC4 instance.

Signed-off-by: Eric Anholt 
---
 drivers/gpu/drm/vc4/vc4_kms.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index ad7925a9e0ea..237a504f11f0 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -230,10 +230,12 @@ int vc4_kms_load(struct drm_device *dev)
 
drm_mode_config_reset(dev);
 
-   vc4->fbdev = drm_fbdev_cma_init(dev, 32,
-   dev->mode_config.num_connector);
-   if (IS_ERR(vc4->fbdev))
-   vc4->fbdev = NULL;
+   if (dev->mode_config.num_connector) {
+   vc4->fbdev = drm_fbdev_cma_init(dev, 32,
+   dev->mode_config.num_connector);
+   if (IS_ERR(vc4->fbdev))
+   vc4->fbdev = NULL;
+   }
 
drm_kms_helper_poll_init(dev);
 
-- 
2.11.0

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