diff mbox

drm/pl111: Fix regression from fbdev CMA helper

Message ID 20180212124019.25268-1-linus.walleij@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Linus Walleij Feb. 12, 2018, 12:40 p.m. UTC
Commit d18df744de1a ("drm/pl111: Use drm_fb_cma_fbdev_init/fini()")
refactored the driver to use fbdev handling in the core, but
mistakedly limited the number of maximum connections to 0.

Predictably, zero possible connections also gives zero
framebuffers when using the system. So let's bump it to 1 so
we get our framebuffer back.

Cc: Noralf Trønnes <noralf@tronnes.org>
Cc: Eric Anholt <eric@anholt.net>
Fixes: d18df744de1a ("drm/pl111: Use drm_fb_cma_fbdev_init/fini()")
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpu/drm/pl111/pl111_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Noralf Trønnes Feb. 12, 2018, 1:42 p.m. UTC | #1
Den 12.02.2018 13.40, skrev Linus Walleij:
> Commit d18df744de1a ("drm/pl111: Use drm_fb_cma_fbdev_init/fini()")
> refactored the driver to use fbdev handling in the core, but
> mistakedly limited the number of maximum connections to 0.
>
> Predictably, zero possible connections also gives zero
> framebuffers when using the system. So let's bump it to 1 so
> we get our framebuffer back.
>
> Cc: Noralf Trønnes <noralf@tronnes.org>
> Cc: Eric Anholt <eric@anholt.net>
> Fixes: d18df744de1a ("drm/pl111: Use drm_fb_cma_fbdev_init/fini()")
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>   drivers/gpu/drm/pl111/pl111_drv.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
> index acb738c69873..306ac7cabd90 100644
> --- a/drivers/gpu/drm/pl111/pl111_drv.c
> +++ b/drivers/gpu/drm/pl111/pl111_drv.c
> @@ -138,7 +138,7 @@ static int pl111_modeset_init(struct drm_device *dev)
>   
>   	drm_mode_config_reset(dev);
>   
> -	drm_fb_cma_fbdev_init(dev, 32, 0);
> +	drm_fb_cma_fbdev_init(dev, 32, 1);
>   
>   	drm_kms_helper_poll_init(dev);
>   

Sorry about that.

I don't know how bridges work, but since it doesn't work this means that
there's no connector available at this point?

This is the arg docs:

  * @max_conn_count: Maximum number of connectors.
  *                  @dev->mode_config.num_connector is used if this is 
zero.

Anyways:
Acked-by: Noralf Trønnes <noralf@tronnes.org>
Noralf Trønnes Feb. 12, 2018, 2:01 p.m. UTC | #2
Den 12.02.2018 14.42, skrev Noralf Trønnes:
>
> Den 12.02.2018 13.40, skrev Linus Walleij:
>> Commit d18df744de1a ("drm/pl111: Use drm_fb_cma_fbdev_init/fini()")
>> refactored the driver to use fbdev handling in the core, but
>> mistakedly limited the number of maximum connections to 0.
>>
>> Predictably, zero possible connections also gives zero
>> framebuffers when using the system. So let's bump it to 1 so
>> we get our framebuffer back.
>>
>> Cc: Noralf Trønnes <noralf@tronnes.org>
>> Cc: Eric Anholt <eric@anholt.net>
>> Fixes: d18df744de1a ("drm/pl111: Use drm_fb_cma_fbdev_init/fini()")
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>> ---
>>   drivers/gpu/drm/pl111/pl111_drv.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/pl111/pl111_drv.c 
>> b/drivers/gpu/drm/pl111/pl111_drv.c
>> index acb738c69873..306ac7cabd90 100644
>> --- a/drivers/gpu/drm/pl111/pl111_drv.c
>> +++ b/drivers/gpu/drm/pl111/pl111_drv.c
>> @@ -138,7 +138,7 @@ static int pl111_modeset_init(struct drm_device 
>> *dev)
>>         drm_mode_config_reset(dev);
>>   -    drm_fb_cma_fbdev_init(dev, 32, 0);
>> +    drm_fb_cma_fbdev_init(dev, 32, 1);
>>         drm_kms_helper_poll_init(dev);
>
> Sorry about that.
>
> I don't know how bridges work, but since it doesn't work this means that
> there's no connector available at this point?
>
> This is the arg docs:
>
>  * @max_conn_count: Maximum number of connectors.
>  *                  @dev->mode_config.num_connector is used if this is 
> zero.
>
> Anyways:
> Acked-by: Noralf Trønnes <noralf@tronnes.org>
>

I forgot to look at the change, and I don't understand how this broke 
anything,

drm/pl111: Use drm_fb_cma_fbdev_init/fini():

@@ -137,8 +138,7 @@ static int pl111_modeset_init(struct drm_device *dev)

      drm_mode_config_reset(dev);

-    priv->fbdev = drm_fbdev_cma_init(dev, 32,
-                     dev->mode_config.num_connector);
+    drm_fb_cma_fbdev_init(dev, 32, 0);

      drm_kms_helper_poll_init(dev);

New max_conn_count setting:

int drm_fb_cma_fbdev_init(struct drm_device *dev, unsigned int 
preferred_bpp,
               unsigned int max_conn_count)
{
     return drm_fb_cma_fbdev_init_with_funcs(dev, preferred_bpp,
                         max_conn_count, NULL);
}

int drm_fb_cma_fbdev_init_with_funcs(struct drm_device *dev,
     unsigned int preferred_bpp, unsigned int max_conn_count,
     const struct drm_framebuffer_funcs *funcs)
{
...
     if (!max_conn_count)
         max_conn_count = dev->mode_config.num_connector;

Noralf.
Linus Walleij Feb. 12, 2018, 3:53 p.m. UTC | #3
On Mon, Feb 12, 2018 at 3:01 PM, Noralf Trønnes <noralf@tronnes.org> wrote:
> Den 12.02.2018 14.42, skrev Noralf Trønnes:
>> Den 12.02.2018 13.40, skrev Linus Walleij:

>>>         drm_mode_config_reset(dev);
>>>   -    drm_fb_cma_fbdev_init(dev, 32, 0);
>>> +    drm_fb_cma_fbdev_init(dev, 32, 1);
>>>         drm_kms_helper_poll_init(dev);

(...)

> I forgot to look at the change, and I don't understand how this broke
> anything,

My wrong.

I had some old codepaths in my out-of-tree stuff (sorry
the only thing that makes the hardware work...).

I was using the old drm_fbdev_cma_init() which isn't
as helpful as the new drm_fb_cma_fbdev_init() but
as the old function is still there it all just "worked"
except for this...

Theres is some confusion with these similarly named
functions :D

Sorry for the fuzz, I fixed my patch set to use the new
function now.

Let's just drop this patch.

Yours,
Linus Walleij
Noralf Trønnes Feb. 12, 2018, 4:02 p.m. UTC | #4
Den 12.02.2018 16.53, skrev Linus Walleij:
> On Mon, Feb 12, 2018 at 3:01 PM, Noralf Trønnes <noralf@tronnes.org> wrote:
>> Den 12.02.2018 14.42, skrev Noralf Trønnes:
>>> Den 12.02.2018 13.40, skrev Linus Walleij:
>>>>          drm_mode_config_reset(dev);
>>>>    -    drm_fb_cma_fbdev_init(dev, 32, 0);
>>>> +    drm_fb_cma_fbdev_init(dev, 32, 1);
>>>>          drm_kms_helper_poll_init(dev);
> (...)
>
>> I forgot to look at the change, and I don't understand how this broke
>> anything,
> My wrong.
>
> I had some old codepaths in my out-of-tree stuff (sorry
> the only thing that makes the hardware work...).
>
> I was using the old drm_fbdev_cma_init() which isn't
> as helpful as the new drm_fb_cma_fbdev_init() but
> as the old function is still there it all just "worked"
> except for this...
>
> Theres is some confusion with these similarly named
> functions :D

Yeah, that's my doing :-/
I have yet to do the final cleanup to remove struct drm_fbdev_cma.
If the generic fbdev emulation that I'm working on turns out good,
then it's possible to entirely drop fbdev code from many drivers.
Hence the waiting.

> Sorry for the fuzz, I fixed my patch set to use the new
> function now.

No problem.

Noralf.

>
> Let's just drop this patch.
>
> Yours,
> Linus Walleij
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
index acb738c69873..306ac7cabd90 100644
--- a/drivers/gpu/drm/pl111/pl111_drv.c
+++ b/drivers/gpu/drm/pl111/pl111_drv.c
@@ -138,7 +138,7 @@  static int pl111_modeset_init(struct drm_device *dev)
 
 	drm_mode_config_reset(dev);
 
-	drm_fb_cma_fbdev_init(dev, 32, 0);
+	drm_fb_cma_fbdev_init(dev, 32, 1);
 
 	drm_kms_helper_poll_init(dev);