Patchwork Re: [PATCH] drm/mediatek: Add MTK Framebuffer-Device (mt7623)

login
register
mail settings
Submitter CK Hu
Date Jan. 11, 2019, 3:20 a.m.
Message ID <1547176809.12054.11.camel@mtksdaap41>
Download mbox | patch
Permalink /patch/697403/
State New
Headers show

Comments

CK Hu - Jan. 11, 2019, 3:20 a.m.
Hi, Daniel:

On Thu, 2019-01-10 at 21:02 +0100, Daniel Vetter wrote:
> On Thu, Jan 10, 2019 at 08:01:37PM +0100, Frank Wunderlich wrote:
> > Hi Daniel,
> > 
> > > > Would be good to use the new generic fbdev emulation code here, for even
> > > > less code. Or at least know why this isn't possible to use for mtk (and
> > > > maybe address that in the core code). Hand-rolling fbdev code shouldn't be
> > > > needed anymore.
> > > 
> > > Back on the mailing list, no private replies please:
> > 
> > i don't wanted to spam all people with dumb questions ;)
> 
> There's no dumb questions, only insufficient documentation :-)
> 
> > > For examples please grep for drm_fbdev_generic_setup(). There's also a
> > > still in-flight series from Gerd Hoffmann to convert over bochs. That,
> > > plus all the kerneldoc linked from there should get you started.
> > > -Daniel
> > 
> > this is one of google best founds if i search for drm_fbdev_generic_setup:
> > 
> > https://lkml.org/lkml/2018/12/19/305
> > 
> > not very helpful...
> > 
> > so i tried kernel-doc
> > 
> > https://www.kernel.org/doc/html/latest/gpu/drm-kms-helpers.html?highlight=drm_fbdev_generic_setup#c.drm_fbdev_generic_setup
> > 
> > which is nice function-reference but i've found no generic workflow
> > 
> > as the posted driver is "only" a driver ported from kernel 4.4 by Alexander, i don't know if this new framework can be used and which parts need to be changed. I only try to bring his code Mainline....
> > Maybe CK Hu can help here because driver is originally from him and he knows internals. Or maybe you can help here?
> > 
> > i personally make my first steps as spare-time kernel-developer :)
> 
> There's a ton of in-kernel users of that function already, I meant you can
> use those to serve as examples ... If those + the kerneldoc aren't
> good enough, then we need to improve them.
> -Daniel

I've tried with drm_fbdev_generic_setup() and it works fine with simple
modification. The patch is

+}

Current drm_fb_helper depends on drm_client to create fbdev. When
drm_client create drm_client_buffer, it need to vmap to get kernel
vaddr. But I think for fbdev, the vaddr is useless. Do you agree that I
temporarily implement .gem_prime_vmap in such way?

Regards,
CK
Daniel Vetter - Jan. 11, 2019, 9:20 a.m.
On Fri, Jan 11, 2019 at 11:20:09AM +0800, CK Hu wrote:
> Hi, Daniel:
> 
> On Thu, 2019-01-10 at 21:02 +0100, Daniel Vetter wrote:
> > On Thu, Jan 10, 2019 at 08:01:37PM +0100, Frank Wunderlich wrote:
> > > Hi Daniel,
> > > 
> > > > > Would be good to use the new generic fbdev emulation code here, for even
> > > > > less code. Or at least know why this isn't possible to use for mtk (and
> > > > > maybe address that in the core code). Hand-rolling fbdev code shouldn't be
> > > > > needed anymore.
> > > > 
> > > > Back on the mailing list, no private replies please:
> > > 
> > > i don't wanted to spam all people with dumb questions ;)
> > 
> > There's no dumb questions, only insufficient documentation :-)
> > 
> > > > For examples please grep for drm_fbdev_generic_setup(). There's also a
> > > > still in-flight series from Gerd Hoffmann to convert over bochs. That,
> > > > plus all the kerneldoc linked from there should get you started.
> > > > -Daniel
> > > 
> > > this is one of google best founds if i search for drm_fbdev_generic_setup:
> > > 
> > > https://lkml.org/lkml/2018/12/19/305
> > > 
> > > not very helpful...
> > > 
> > > so i tried kernel-doc
> > > 
> > > https://www.kernel.org/doc/html/latest/gpu/drm-kms-helpers.html?highlight=drm_fbdev_generic_setup#c.drm_fbdev_generic_setup
> > > 
> > > which is nice function-reference but i've found no generic workflow
> > > 
> > > as the posted driver is "only" a driver ported from kernel 4.4 by Alexander, i don't know if this new framework can be used and which parts need to be changed. I only try to bring his code Mainline....
> > > Maybe CK Hu can help here because driver is originally from him and he knows internals. Or maybe you can help here?
> > > 
> > > i personally make my first steps as spare-time kernel-developer :)
> > 
> > There's a ton of in-kernel users of that function already, I meant you can
> > use those to serve as examples ... If those + the kerneldoc aren't
> > good enough, then we need to improve them.
> > -Daniel
> 
> I've tried with drm_fbdev_generic_setup() and it works fine with simple
> modification. The patch is
> 
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -16,6 +16,7 @@
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_crtc_helper.h>
> +#include <drm/drm_fb_helper.h>
>  #include <drm/drm_gem.h>
>  #include <drm/drm_gem_cma_helper.h>
>  #include <drm/drm_of.h>
> @@ -379,6 +380,7 @@ static void mtk_drm_kms_deinit(struct drm_device
> *drm)
>         .gem_prime_get_sg_table = mtk_gem_prime_get_sg_table,
>         .gem_prime_import_sg_table = mtk_gem_prime_import_sg_table,
>         .gem_prime_mmap = mtk_drm_gem_mmap_buf,
> +       .gem_prime_vmap = mtk_drm_gem_prime_vmap,
>         .ioctls = mtk_ioctls,
>         .num_ioctls = ARRAY_SIZE(mtk_ioctls),
>         .fops = &mtk_drm_fops,
> @@ -416,6 +418,8 @@ static int mtk_drm_bind(struct device *dev)
>         if (ret < 0)
>                 goto err_deinit;
> 
> +       drm_fbdev_generic_setup(drm, 32);
> +
>         return 0;
> 
> 
> But I implement .gem_prime_vmap with a workaround,
> 
> 
> --- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.c
> @@ -280,3 +280,8 @@ int mtk_gem_create_ioctl(struct drm_device *dev,
> void *data,
>         mtk_drm_gem_free_object(&mtk_gem->base);
>         return ret;
>  }
> +
> +void *mtk_drm_gem_prime_vmap(struct drm_gem_object *obj)
> +{
> +       return (void *)1;
> +}
> 
> Current drm_fb_helper depends on drm_client to create fbdev. When
> drm_client create drm_client_buffer, it need to vmap to get kernel
> vaddr. But I think for fbdev, the vaddr is useless. Do you agree that I
> temporarily implement .gem_prime_vmap in such way?

The vmap is used by fbcon, without that fbdev won't really work I think.
So I'm rather confused on what you tested ...

Adding Noralf, he's the expert here.
-Daniel
CK Hu - Jan. 11, 2019, 9:49 a.m.
Hi, Daniel:

On Fri, 2019-01-11 at 10:20 +0100, Daniel Vetter wrote:
> On Fri, Jan 11, 2019 at 11:20:09AM +0800, CK Hu wrote:
> > Hi, Daniel:
> > 
> > On Thu, 2019-01-10 at 21:02 +0100, Daniel Vetter wrote:
> > > On Thu, Jan 10, 2019 at 08:01:37PM +0100, Frank Wunderlich wrote:
> > > > Hi Daniel,
> > > > 
> > > > > > Would be good to use the new generic fbdev emulation code here, for even
> > > > > > less code. Or at least know why this isn't possible to use for mtk (and
> > > > > > maybe address that in the core code). Hand-rolling fbdev code shouldn't be
> > > > > > needed anymore.
> > > > > 
> > > > > Back on the mailing list, no private replies please:
> > > > 
> > > > i don't wanted to spam all people with dumb questions ;)
> > > 
> > > There's no dumb questions, only insufficient documentation :-)
> > > 
> > > > > For examples please grep for drm_fbdev_generic_setup(). There's also a
> > > > > still in-flight series from Gerd Hoffmann to convert over bochs. That,
> > > > > plus all the kerneldoc linked from there should get you started.
> > > > > -Daniel
> > > > 
> > > > this is one of google best founds if i search for drm_fbdev_generic_setup:
> > > > 
> > > > https://lkml.org/lkml/2018/12/19/305
> > > > 
> > > > not very helpful...
> > > > 
> > > > so i tried kernel-doc
> > > > 
> > > > https://www.kernel.org/doc/html/latest/gpu/drm-kms-helpers.html?highlight=drm_fbdev_generic_setup#c.drm_fbdev_generic_setup
> > > > 
> > > > which is nice function-reference but i've found no generic workflow
> > > > 
> > > > as the posted driver is "only" a driver ported from kernel 4.4 by Alexander, i don't know if this new framework can be used and which parts need to be changed. I only try to bring his code Mainline....
> > > > Maybe CK Hu can help here because driver is originally from him and he knows internals. Or maybe you can help here?
> > > > 
> > > > i personally make my first steps as spare-time kernel-developer :)
> > > 
> > > There's a ton of in-kernel users of that function already, I meant you can
> > > use those to serve as examples ... If those + the kerneldoc aren't
> > > good enough, then we need to improve them.
> > > -Daniel
> > 
> > I've tried with drm_fbdev_generic_setup() and it works fine with simple
> > modification. The patch is
> > 
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > @@ -16,6 +16,7 @@
> >  #include <drm/drm_atomic.h>
> >  #include <drm/drm_atomic_helper.h>
> >  #include <drm/drm_crtc_helper.h>
> > +#include <drm/drm_fb_helper.h>
> >  #include <drm/drm_gem.h>
> >  #include <drm/drm_gem_cma_helper.h>
> >  #include <drm/drm_of.h>
> > @@ -379,6 +380,7 @@ static void mtk_drm_kms_deinit(struct drm_device
> > *drm)
> >         .gem_prime_get_sg_table = mtk_gem_prime_get_sg_table,
> >         .gem_prime_import_sg_table = mtk_gem_prime_import_sg_table,
> >         .gem_prime_mmap = mtk_drm_gem_mmap_buf,
> > +       .gem_prime_vmap = mtk_drm_gem_prime_vmap,
> >         .ioctls = mtk_ioctls,
> >         .num_ioctls = ARRAY_SIZE(mtk_ioctls),
> >         .fops = &mtk_drm_fops,
> > @@ -416,6 +418,8 @@ static int mtk_drm_bind(struct device *dev)
> >         if (ret < 0)
> >                 goto err_deinit;
> > 
> > +       drm_fbdev_generic_setup(drm, 32);
> > +
> >         return 0;
> > 
> > 
> > But I implement .gem_prime_vmap with a workaround,
> > 
> > 
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.c
> > @@ -280,3 +280,8 @@ int mtk_gem_create_ioctl(struct drm_device *dev,
> > void *data,
> >         mtk_drm_gem_free_object(&mtk_gem->base);
> >         return ret;
> >  }
> > +
> > +void *mtk_drm_gem_prime_vmap(struct drm_gem_object *obj)
> > +{
> > +       return (void *)1;
> > +}
> > 
> > Current drm_fb_helper depends on drm_client to create fbdev. When
> > drm_client create drm_client_buffer, it need to vmap to get kernel
> > vaddr. But I think for fbdev, the vaddr is useless. Do you agree that I
> > temporarily implement .gem_prime_vmap in such way?
> 
> The vmap is used by fbcon, without that fbdev won't really work I think.
> So I'm rather confused on what you tested ...
> 
> Adding Noralf, he's the expert here.
> -Daniel

I test with fb_test [1], it is a user space program just open /dev/fb0
and mmap it to user space. I does not turn on fbcon so everything looks
well. If support fbcon is essential, I would implement vmap correctly.
If it's not essential, I think I could return 0 when
CONFIG_FRAMBUFFER_CONSOLE is defined.

Regards,
CK

[1]
https://android.googlesource.com/platform/system/extras/+/master/tests/framebuffer/fb_test.c
Daniel Vetter - Jan. 11, 2019, 10:25 a.m.
On Fri, Jan 11, 2019 at 05:49:15PM +0800, CK Hu wrote:
> Hi, Daniel:
> 
> On Fri, 2019-01-11 at 10:20 +0100, Daniel Vetter wrote:
> > On Fri, Jan 11, 2019 at 11:20:09AM +0800, CK Hu wrote:
> > > Hi, Daniel:
> > > 
> > > On Thu, 2019-01-10 at 21:02 +0100, Daniel Vetter wrote:
> > > > On Thu, Jan 10, 2019 at 08:01:37PM +0100, Frank Wunderlich wrote:
> > > > > Hi Daniel,
> > > > > 
> > > > > > > Would be good to use the new generic fbdev emulation code here, for even
> > > > > > > less code. Or at least know why this isn't possible to use for mtk (and
> > > > > > > maybe address that in the core code). Hand-rolling fbdev code shouldn't be
> > > > > > > needed anymore.
> > > > > > 
> > > > > > Back on the mailing list, no private replies please:
> > > > > 
> > > > > i don't wanted to spam all people with dumb questions ;)
> > > > 
> > > > There's no dumb questions, only insufficient documentation :-)
> > > > 
> > > > > > For examples please grep for drm_fbdev_generic_setup(). There's also a
> > > > > > still in-flight series from Gerd Hoffmann to convert over bochs. That,
> > > > > > plus all the kerneldoc linked from there should get you started.
> > > > > > -Daniel
> > > > > 
> > > > > this is one of google best founds if i search for drm_fbdev_generic_setup:
> > > > > 
> > > > > https://lkml.org/lkml/2018/12/19/305
> > > > > 
> > > > > not very helpful...
> > > > > 
> > > > > so i tried kernel-doc
> > > > > 
> > > > > https://www.kernel.org/doc/html/latest/gpu/drm-kms-helpers.html?highlight=drm_fbdev_generic_setup#c.drm_fbdev_generic_setup
> > > > > 
> > > > > which is nice function-reference but i've found no generic workflow
> > > > > 
> > > > > as the posted driver is "only" a driver ported from kernel 4.4 by Alexander, i don't know if this new framework can be used and which parts need to be changed. I only try to bring his code Mainline....
> > > > > Maybe CK Hu can help here because driver is originally from him and he knows internals. Or maybe you can help here?
> > > > > 
> > > > > i personally make my first steps as spare-time kernel-developer :)
> > > > 
> > > > There's a ton of in-kernel users of that function already, I meant you can
> > > > use those to serve as examples ... If those + the kerneldoc aren't
> > > > good enough, then we need to improve them.
> > > > -Daniel
> > > 
> > > I've tried with drm_fbdev_generic_setup() and it works fine with simple
> > > modification. The patch is
> > > 
> > > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > > @@ -16,6 +16,7 @@
> > >  #include <drm/drm_atomic.h>
> > >  #include <drm/drm_atomic_helper.h>
> > >  #include <drm/drm_crtc_helper.h>
> > > +#include <drm/drm_fb_helper.h>
> > >  #include <drm/drm_gem.h>
> > >  #include <drm/drm_gem_cma_helper.h>
> > >  #include <drm/drm_of.h>
> > > @@ -379,6 +380,7 @@ static void mtk_drm_kms_deinit(struct drm_device
> > > *drm)
> > >         .gem_prime_get_sg_table = mtk_gem_prime_get_sg_table,
> > >         .gem_prime_import_sg_table = mtk_gem_prime_import_sg_table,
> > >         .gem_prime_mmap = mtk_drm_gem_mmap_buf,
> > > +       .gem_prime_vmap = mtk_drm_gem_prime_vmap,
> > >         .ioctls = mtk_ioctls,
> > >         .num_ioctls = ARRAY_SIZE(mtk_ioctls),
> > >         .fops = &mtk_drm_fops,
> > > @@ -416,6 +418,8 @@ static int mtk_drm_bind(struct device *dev)
> > >         if (ret < 0)
> > >                 goto err_deinit;
> > > 
> > > +       drm_fbdev_generic_setup(drm, 32);
> > > +
> > >         return 0;
> > > 
> > > 
> > > But I implement .gem_prime_vmap with a workaround,
> > > 
> > > 
> > > --- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.c
> > > @@ -280,3 +280,8 @@ int mtk_gem_create_ioctl(struct drm_device *dev,
> > > void *data,
> > >         mtk_drm_gem_free_object(&mtk_gem->base);
> > >         return ret;
> > >  }
> > > +
> > > +void *mtk_drm_gem_prime_vmap(struct drm_gem_object *obj)
> > > +{
> > > +       return (void *)1;
> > > +}
> > > 
> > > Current drm_fb_helper depends on drm_client to create fbdev. When
> > > drm_client create drm_client_buffer, it need to vmap to get kernel
> > > vaddr. But I think for fbdev, the vaddr is useless. Do you agree that I
> > > temporarily implement .gem_prime_vmap in such way?
> > 
> > The vmap is used by fbcon, without that fbdev won't really work I think.
> > So I'm rather confused on what you tested ...
> > 
> > Adding Noralf, he's the expert here.
> > -Daniel
> 
> I test with fb_test [1], it is a user space program just open /dev/fb0
> and mmap it to user space. I does not turn on fbcon so everything looks
> well. If support fbcon is essential, I would implement vmap correctly.
> If it's not essential, I think I could return 0 when
> CONFIG_FRAMBUFFER_CONSOLE is defined.

I think fbdev emulation without working fbcon is fairly useless. And it
shouldn't be that much work to make it work, we have quite a few more
helers for gem bo nowadays.
-Daniel

> 
> Regards,
> CK
> 
> [1]
> https://android.googlesource.com/platform/system/extras/+/master/tests/framebuffer/fb_test.c
> 
>
Noralf Trønnes - Jan. 11, 2019, 4:07 p.m.
Den 11.01.2019 11.25, skrev Daniel Vetter:
> On Fri, Jan 11, 2019 at 05:49:15PM +0800, CK Hu wrote:
>> Hi, Daniel:
>>
>> On Fri, 2019-01-11 at 10:20 +0100, Daniel Vetter wrote:
>>> On Fri, Jan 11, 2019 at 11:20:09AM +0800, CK Hu wrote:
>>>> Hi, Daniel:
>>>>
>>>> On Thu, 2019-01-10 at 21:02 +0100, Daniel Vetter wrote:
>>>>> On Thu, Jan 10, 2019 at 08:01:37PM +0100, Frank Wunderlich wrote:
>>>>>> Hi Daniel,
>>>>>>
>>>>>>>> Would be good to use the new generic fbdev emulation code here, for even
>>>>>>>> less code. Or at least know why this isn't possible to use for mtk (and
>>>>>>>> maybe address that in the core code). Hand-rolling fbdev code shouldn't be
>>>>>>>> needed anymore.
>>>>>>>
>>>>>>> Back on the mailing list, no private replies please:
>>>>>>
>>>>>> i don't wanted to spam all people with dumb questions ;)
>>>>>
>>>>> There's no dumb questions, only insufficient documentation :-)
>>>>>
>>>>>>> For examples please grep for drm_fbdev_generic_setup(). There's also a
>>>>>>> still in-flight series from Gerd Hoffmann to convert over bochs. That,
>>>>>>> plus all the kerneldoc linked from there should get you started.
>>>>>>> -Daniel
>>>>>>
>>>>>> this is one of google best founds if i search for drm_fbdev_generic_setup:
>>>>>>
>>>>>> https://lkml.org/lkml/2018/12/19/305
>>>>>>
>>>>>> not very helpful...
>>>>>>
>>>>>> so i tried kernel-doc
>>>>>>
>>>>>> https://www.kernel.org/doc/html/latest/gpu/drm-kms-helpers.html?highlight=drm_fbdev_generic_setup#c.drm_fbdev_generic_setup
>>>>>>
>>>>>> which is nice function-reference but i've found no generic workflow
>>>>>>
>>>>>> as the posted driver is "only" a driver ported from kernel 4.4 by Alexander, i don't know if this new framework can be used and which parts need to be changed. I only try to bring his code Mainline....
>>>>>> Maybe CK Hu can help here because driver is originally from him and he knows internals. Or maybe you can help here?
>>>>>>
>>>>>> i personally make my first steps as spare-time kernel-developer :)
>>>>>
>>>>> There's a ton of in-kernel users of that function already, I meant you can
>>>>> use those to serve as examples ... If those + the kerneldoc aren't
>>>>> good enough, then we need to improve them.
>>>>> -Daniel
>>>>
>>>> I've tried with drm_fbdev_generic_setup() and it works fine with simple
>>>> modification. The patch is
>>>>
>>>> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
>>>> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
>>>> @@ -16,6 +16,7 @@
>>>>  #include <drm/drm_atomic.h>
>>>>  #include <drm/drm_atomic_helper.h>
>>>>  #include <drm/drm_crtc_helper.h>
>>>> +#include <drm/drm_fb_helper.h>
>>>>  #include <drm/drm_gem.h>
>>>>  #include <drm/drm_gem_cma_helper.h>
>>>>  #include <drm/drm_of.h>
>>>> @@ -379,6 +380,7 @@ static void mtk_drm_kms_deinit(struct drm_device
>>>> *drm)
>>>>         .gem_prime_get_sg_table = mtk_gem_prime_get_sg_table,
>>>>         .gem_prime_import_sg_table = mtk_gem_prime_import_sg_table,
>>>>         .gem_prime_mmap = mtk_drm_gem_mmap_buf,
>>>> +       .gem_prime_vmap = mtk_drm_gem_prime_vmap,
>>>>         .ioctls = mtk_ioctls,
>>>>         .num_ioctls = ARRAY_SIZE(mtk_ioctls),
>>>>         .fops = &mtk_drm_fops,
>>>> @@ -416,6 +418,8 @@ static int mtk_drm_bind(struct device *dev)
>>>>         if (ret < 0)
>>>>                 goto err_deinit;
>>>>
>>>> +       drm_fbdev_generic_setup(drm, 32);
>>>> +
>>>>         return 0;
>>>>
>>>>
>>>> But I implement .gem_prime_vmap with a workaround,
>>>>
>>>>
>>>> --- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c
>>>> +++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.c
>>>> @@ -280,3 +280,8 @@ int mtk_gem_create_ioctl(struct drm_device *dev,
>>>> void *data,
>>>>         mtk_drm_gem_free_object(&mtk_gem->base);
>>>>         return ret;
>>>>  }
>>>> +
>>>> +void *mtk_drm_gem_prime_vmap(struct drm_gem_object *obj)
>>>> +{
>>>> +       return (void *)1;
>>>> +}
>>>>
>>>> Current drm_fb_helper depends on drm_client to create fbdev. When
>>>> drm_client create drm_client_buffer, it need to vmap to get kernel
>>>> vaddr. But I think for fbdev, the vaddr is useless. Do you agree that I
>>>> temporarily implement .gem_prime_vmap in such way?
>>>
>>> The vmap is used by fbcon, without that fbdev won't really work I think.
>>> So I'm rather confused on what you tested ...
>>>
>>> Adding Noralf, he's the expert here.
>>> -Daniel
>>
>> I test with fb_test [1], it is a user space program just open /dev/fb0
>> and mmap it to user space. I does not turn on fbcon so everything looks
>> well. If support fbcon is essential, I would implement vmap correctly.
>> If it's not essential, I think I could return 0 when
>> CONFIG_FRAMBUFFER_CONSOLE is defined.
> 
> I think fbdev emulation without working fbcon is fairly useless. And it
> shouldn't be that much work to make it work, we have quite a few more
> helers for gem bo nowadays.

If the virtual address is not set, fbcon can't be used and that means it
has to be disabled using something like this in the driver Kconfig:
	depends on !FRAMEBUFFER_CONSOLE

Otherwise it will crash if someone tries to use it.

The problem here is that this driver doesn't map a virtual address for
its buffers:
	mtk_gem->dma_attrs |= DMA_ATTR_NO_KERNEL_MAPPING;

Probably because of limited virtual address space.
rockchip is in the same situation.

I'm aware of this shortcoming of the generic emulation, but I don't see
how it can be solved without using somekind of flag attached to the dumb
buffer creation request.

Noralf.
Frank Wunderlich - Jan. 11, 2019, 6:23 p.m.
like Daniel guessed, fb-console does not work with this Patch.

i tried your Patch (additional patch for headerfile): https://github.com/frank-w/BPI-R2-4.14/commits/4.20-fbdev

and R2 hangs  (no crash and no further boot)

[    5.435075] mediatek-drm 14000000.dispsys: bound 14012000.rdma (ops mtk_disp)
[    5.435134] mediatek-drm 14000000.dispsys: bound 14014000.dpi (ops mtk_dpi_c)
[    5.459277] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).      
[    5.465863] [drm] No driver support for vblank timestamp query.              
[    5.472331] [drm] Initialized mediatek 1.0.0 20150513 for 14000000.dispsys o0

my monitor is switched on, but no output....we had added fbdev-driver to get FB-console over hdmi, so it's currently useless

regards Frank
CK Hu - Jan. 14, 2019, 2:02 a.m.
On Fri, 2019-01-11 at 17:07 +0100, Noralf Trønnes wrote:
> 
> Den 11.01.2019 11.25, skrev Daniel Vetter:
> > On Fri, Jan 11, 2019 at 05:49:15PM +0800, CK Hu wrote:
> >> Hi, Daniel:
> >>
> >> On Fri, 2019-01-11 at 10:20 +0100, Daniel Vetter wrote:
> >>> On Fri, Jan 11, 2019 at 11:20:09AM +0800, CK Hu wrote:
> >>>> Hi, Daniel:
> >>>>
> >>>> On Thu, 2019-01-10 at 21:02 +0100, Daniel Vetter wrote:
> >>>>> On Thu, Jan 10, 2019 at 08:01:37PM +0100, Frank Wunderlich wrote:
> >>>>>> Hi Daniel,
> >>>>>>
> >>>>>>>> Would be good to use the new generic fbdev emulation code here, for even
> >>>>>>>> less code. Or at least know why this isn't possible to use for mtk (and
> >>>>>>>> maybe address that in the core code). Hand-rolling fbdev code shouldn't be
> >>>>>>>> needed anymore.
> >>>>>>>
> >>>>>>> Back on the mailing list, no private replies please:
> >>>>>>
> >>>>>> i don't wanted to spam all people with dumb questions ;)
> >>>>>
> >>>>> There's no dumb questions, only insufficient documentation :-)
> >>>>>
> >>>>>>> For examples please grep for drm_fbdev_generic_setup(). There's also a
> >>>>>>> still in-flight series from Gerd Hoffmann to convert over bochs. That,
> >>>>>>> plus all the kerneldoc linked from there should get you started.
> >>>>>>> -Daniel
> >>>>>>
> >>>>>> this is one of google best founds if i search for drm_fbdev_generic_setup:
> >>>>>>
> >>>>>> https://lkml.org/lkml/2018/12/19/305
> >>>>>>
> >>>>>> not very helpful...
> >>>>>>
> >>>>>> so i tried kernel-doc
> >>>>>>
> >>>>>> https://www.kernel.org/doc/html/latest/gpu/drm-kms-helpers.html?highlight=drm_fbdev_generic_setup#c.drm_fbdev_generic_setup
> >>>>>>
> >>>>>> which is nice function-reference but i've found no generic workflow
> >>>>>>
> >>>>>> as the posted driver is "only" a driver ported from kernel 4.4 by Alexander, i don't know if this new framework can be used and which parts need to be changed. I only try to bring his code Mainline....
> >>>>>> Maybe CK Hu can help here because driver is originally from him and he knows internals. Or maybe you can help here?
> >>>>>>
> >>>>>> i personally make my first steps as spare-time kernel-developer :)
> >>>>>
> >>>>> There's a ton of in-kernel users of that function already, I meant you can
> >>>>> use those to serve as examples ... If those + the kerneldoc aren't
> >>>>> good enough, then we need to improve them.
> >>>>> -Daniel
> >>>>
> >>>> I've tried with drm_fbdev_generic_setup() and it works fine with simple
> >>>> modification. The patch is
> >>>>
> >>>> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> >>>> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> >>>> @@ -16,6 +16,7 @@
> >>>>  #include <drm/drm_atomic.h>
> >>>>  #include <drm/drm_atomic_helper.h>
> >>>>  #include <drm/drm_crtc_helper.h>
> >>>> +#include <drm/drm_fb_helper.h>
> >>>>  #include <drm/drm_gem.h>
> >>>>  #include <drm/drm_gem_cma_helper.h>
> >>>>  #include <drm/drm_of.h>
> >>>> @@ -379,6 +380,7 @@ static void mtk_drm_kms_deinit(struct drm_device
> >>>> *drm)
> >>>>         .gem_prime_get_sg_table = mtk_gem_prime_get_sg_table,
> >>>>         .gem_prime_import_sg_table = mtk_gem_prime_import_sg_table,
> >>>>         .gem_prime_mmap = mtk_drm_gem_mmap_buf,
> >>>> +       .gem_prime_vmap = mtk_drm_gem_prime_vmap,
> >>>>         .ioctls = mtk_ioctls,
> >>>>         .num_ioctls = ARRAY_SIZE(mtk_ioctls),
> >>>>         .fops = &mtk_drm_fops,
> >>>> @@ -416,6 +418,8 @@ static int mtk_drm_bind(struct device *dev)
> >>>>         if (ret < 0)
> >>>>                 goto err_deinit;
> >>>>
> >>>> +       drm_fbdev_generic_setup(drm, 32);
> >>>> +
> >>>>         return 0;
> >>>>
> >>>>
> >>>> But I implement .gem_prime_vmap with a workaround,
> >>>>
> >>>>
> >>>> --- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c
> >>>> +++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.c
> >>>> @@ -280,3 +280,8 @@ int mtk_gem_create_ioctl(struct drm_device *dev,
> >>>> void *data,
> >>>>         mtk_drm_gem_free_object(&mtk_gem->base);
> >>>>         return ret;
> >>>>  }
> >>>> +
> >>>> +void *mtk_drm_gem_prime_vmap(struct drm_gem_object *obj)
> >>>> +{
> >>>> +       return (void *)1;
> >>>> +}
> >>>>
> >>>> Current drm_fb_helper depends on drm_client to create fbdev. When
> >>>> drm_client create drm_client_buffer, it need to vmap to get kernel
> >>>> vaddr. But I think for fbdev, the vaddr is useless. Do you agree that I
> >>>> temporarily implement .gem_prime_vmap in such way?
> >>>
> >>> The vmap is used by fbcon, without that fbdev won't really work I think.
> >>> So I'm rather confused on what you tested ...
> >>>
> >>> Adding Noralf, he's the expert here.
> >>> -Daniel
> >>
> >> I test with fb_test [1], it is a user space program just open /dev/fb0
> >> and mmap it to user space. I does not turn on fbcon so everything looks
> >> well. If support fbcon is essential, I would implement vmap correctly.
> >> If it's not essential, I think I could return 0 when
> >> CONFIG_FRAMBUFFER_CONSOLE is defined.
> > 
> > I think fbdev emulation without working fbcon is fairly useless. And it
> > shouldn't be that much work to make it work, we have quite a few more
> > helers for gem bo nowadays.
> 
> If the virtual address is not set, fbcon can't be used and that means it
> has to be disabled using something like this in the driver Kconfig:
> 	depends on !FRAMEBUFFER_CONSOLE
> 
> Otherwise it will crash if someone tries to use it.
> 
> The problem here is that this driver doesn't map a virtual address for
> its buffers:
> 	mtk_gem->dma_attrs |= DMA_ATTR_NO_KERNEL_MAPPING;
> 
> Probably because of limited virtual address space.
> rockchip is in the same situation.
> 
> I'm aware of this shortcoming of the generic emulation, but I don't see
> how it can be solved without using somekind of flag attached to the dumb
> buffer creation request.

The virtual address space is limited so we do not map it by default. I
also see the shortcoming of the generic emulation, so I would refer to
rockchip to implement this mapping.

Regards,
CK

> 
> Noralf.

Patch

--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -16,6 +16,7 @@ 
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_fb_helper.h>
 #include <drm/drm_gem.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_of.h>
@@ -379,6 +380,7 @@  static void mtk_drm_kms_deinit(struct drm_device
*drm)
        .gem_prime_get_sg_table = mtk_gem_prime_get_sg_table,
        .gem_prime_import_sg_table = mtk_gem_prime_import_sg_table,
        .gem_prime_mmap = mtk_drm_gem_mmap_buf,
+       .gem_prime_vmap = mtk_drm_gem_prime_vmap,
        .ioctls = mtk_ioctls,
        .num_ioctls = ARRAY_SIZE(mtk_ioctls),
        .fops = &mtk_drm_fops,
@@ -416,6 +418,8 @@  static int mtk_drm_bind(struct device *dev)
        if (ret < 0)
                goto err_deinit;

+       drm_fbdev_generic_setup(drm, 32);
+
        return 0;


But I implement .gem_prime_vmap with a workaround,


--- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.c
@@ -280,3 +280,8 @@  int mtk_gem_create_ioctl(struct drm_device *dev,
void *data,
        mtk_drm_gem_free_object(&mtk_gem->base);
        return ret;
 }
+
+void *mtk_drm_gem_prime_vmap(struct drm_gem_object *obj)
+{
+       return (void *)1;