re: [PATCH 0/2] drm: add SimpleDRM driver

re: [PATCH 0/2] drm: add SimpleDRM driver

[Ken Phillis Jr]

[-- Attachment #1.1: Type: text/plain, Size: 869 bytes --]

I believe this driver is extremely useful, and I see possible issues with
the fact that the driver is GPL Only. This driver is critical for devices
that lack a proper DRM driver, and locking it into GPL Only could lead to
issues with porting the system over to platforms like FreeBSD,
DragonFlyBSD, etc.  Is there any chance the code placed in the
"drivers/gpu/drm/simpledrm" folder be placed under a more permissive
license setup. I would suggest using MIT[1], BSD 2-Clause[2], or BSD
3-clause[3]. Also as a quick note it is possible to enable this under a
dual-license setup with GPLv2/BSD-3 Clause. An example a dual licensed
driver is the Intel iwlwifi driver ( see:
drivers/net/wireless/intel/iwlwifi/iwl-8000.c )


[1]https://opensource.org/licenses/mit-license.html
[2]https://opensource.org/licenses/BSD-2-Clause
[3]https://opensource.org/licenses/BSD-3-Clause

[-- Attachment #1.2: Type: text/html, Size: 1102 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH 0/2] drm: add SimpleDRM driver

[Daniel Vetter]

On Thu, Aug 04, 2016 at 02:50:37PM -0500, Ken Phillis Jr wrote:
> I believe this driver is extremely useful, and I see possible issues with
> the fact that the driver is GPL Only. This driver is critical for devices
> that lack a proper DRM driver, and locking it into GPL Only could lead to
> issues with porting the system over to platforms like FreeBSD,
> DragonFlyBSD, etc.  Is there any chance the code placed in the
> "drivers/gpu/drm/simpledrm" folder be placed under a more permissive
> license setup. I would suggest using MIT[1], BSD 2-Clause[2], or BSD
> 3-clause[3]. Also as a quick note it is possible to enable this under a
> dual-license setup with GPLv2/BSD-3 Clause. An example a dual licensed
> driver is the Intel iwlwifi driver ( see:
> drivers/net/wireless/intel/iwlwifi/iwl-8000.c )

tbh, most if not all arm drivers are gpl only, and due to code sharing and
refactoring I'd say a lot of that has leaked all over drm. IANAL and all
that, but personally I believe that the entire idea of drm being MIT is
walking on ever thinner ice. And personally I'm not going to extend effort
to slow this down or prevent it outright, since I think all that code
sharing with arm folks is extremely beneficial for everyone. At least here
on Linux.
-Daniel

> 
> 
> [1]https://opensource.org/licenses/mit-license.html
> [2]https://opensource.org/licenses/BSD-2-Clause
> [3]https://opensource.org/licenses/BSD-3-Clause

> _______________________________________________
> 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

Re: [PATCH 0/2] drm: add SimpleDRM driver

[David Herrmann]

Hi

On Thu, Aug 4, 2016 at 10:01 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Aug 04, 2016 at 02:50:37PM -0500, Ken Phillis Jr wrote:
>> I believe this driver is extremely useful, and I see possible issues with
>> the fact that the driver is GPL Only. This driver is critical for devices
>> that lack a proper DRM driver, and locking it into GPL Only could lead to
>> issues with porting the system over to platforms like FreeBSD,
>> DragonFlyBSD, etc.  Is there any chance the code placed in the
>> "drivers/gpu/drm/simpledrm" folder be placed under a more permissive
>> license setup. I would suggest using MIT[1], BSD 2-Clause[2], or BSD
>> 3-clause[3]. Also as a quick note it is possible to enable this under a
>> dual-license setup with GPLv2/BSD-3 Clause. An example a dual licensed
>> driver is the Intel iwlwifi driver ( see:
>> drivers/net/wireless/intel/iwlwifi/iwl-8000.c )
>
> tbh, most if not all arm drivers are gpl only, and due to code sharing and
> refactoring I'd say a lot of that has leaked all over drm. IANAL and all
> that, but personally I believe that the entire idea of drm being MIT is
> walking on ever thinner ice. And personally I'm not going to extend effort
> to slow this down or prevent it outright, since I think all that code
> sharing with arm folks is extremely beneficial for everyone. At least here
> on Linux.
> -Daniel

On top of that: Feel free to copy SimpleDRM in any way possible.
Consider my original implementation public domain.

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

Re: [PATCH 0/2] drm: add SimpleDRM driver

[Ken Phillis Jr]

[-- Attachment #1.1: Type: text/plain, Size: 1274 bytes --]

On Thu, Aug 4, 2016 at 3:01 PM, Daniel Vetter <daniel@ffwll.ch> wrote:

> tbh, most if not all arm drivers are gpl only, and due to code sharing and
> refactoring I'd say a lot of that has leaked all over drm. IANAL and all
> that, but personally I believe that the entire idea of drm being MIT is
> walking on ever thinner ice. And personally I'm not going to extend effort
> to slow this down or prevent it outright, since I think all that code
> sharing with arm folks is extremely beneficial for everyone. At least here
> on Linux.
> -Daniel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
>

I generally do not have a issue with GPLv2 license. I believe that most
developers
are under the impression that for a driver (and code) to get mainlined into
the
Linux kernel requires the code to be placed under GPLv2 only. This is just a
generalized check to make sure that there is no problem placing this code
under a more permissive license.

On Thu, Aug 4, 2016 at 3:07 PM, David Herrmann <dh.herrmann@gmail.com>
wrote:

> Hi
>
> On top of that: Feel free to copy SimpleDRM in any way possible.
> Consider my original implementation public domain.
>
> Thanks
> David
>

Thank you for the quick response, and I'll make a note of this.

[-- Attachment #1.2: Type: text/html, Size: 2197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH 0/2] drm: add SimpleDRM driver

[Jani Nikula]

On Thu, 04 Aug 2016, David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> On Thu, Aug 4, 2016 at 10:01 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Thu, Aug 04, 2016 at 02:50:37PM -0500, Ken Phillis Jr wrote:
>>> I believe this driver is extremely useful, and I see possible issues with
>>> the fact that the driver is GPL Only. This driver is critical for devices
>>> that lack a proper DRM driver, and locking it into GPL Only could lead to
>>> issues with porting the system over to platforms like FreeBSD,
>>> DragonFlyBSD, etc.  Is there any chance the code placed in the
>>> "drivers/gpu/drm/simpledrm" folder be placed under a more permissive
>>> license setup. I would suggest using MIT[1], BSD 2-Clause[2], or BSD
>>> 3-clause[3]. Also as a quick note it is possible to enable this under a
>>> dual-license setup with GPLv2/BSD-3 Clause. An example a dual licensed
>>> driver is the Intel iwlwifi driver ( see:
>>> drivers/net/wireless/intel/iwlwifi/iwl-8000.c )
>>
>> tbh, most if not all arm drivers are gpl only, and due to code sharing and
>> refactoring I'd say a lot of that has leaked all over drm. IANAL and all
>> that, but personally I believe that the entire idea of drm being MIT is
>> walking on ever thinner ice. And personally I'm not going to extend effort
>> to slow this down or prevent it outright, since I think all that code
>> sharing with arm folks is extremely beneficial for everyone. At least here
>> on Linux.
>> -Daniel
>
> On top of that: Feel free to copy SimpleDRM in any way possible.
> Consider my original implementation public domain.

N.b. the improvements by others on top of that, at least once included
in the kernel tree, will not be public domain. IANAL etc.

BR,
Jani.


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

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 0/2] drm: add SimpleDRM driver

[Noralf Trønnes]

This patchset adds the simpledrm driver by David Herrmann based on a
patchset[1] from 2014. That patchset also included patches for kicking
out simpledrm by real drivers. I have stayed away from that since it
involves another subsystem and I would probably be unable to answer any
questions about the implementation.

I have done my best to bring simpledrm up to speed. However I was unable to
loose drm_legacy_mmap() in sdrm_drm_mmap() since I don't understand much of
the gem code.

I was left with some questions after doing this:

- Is there any reason why simpledrm can't use drm_gem_cma_helper?
  One obvious difference is that of contiguous memory allocation:

  sdrm_gem_get_pages():
	obj->pages = drm_malloc_ab(num, sizeof(*obj->pages));
	for (i = 0; i < num; ++i) {
		obj->pages[i] = alloc_page(GFP_KERNEL | __GFP_ZERO);
	obj->vmapping = vmap(obj->pages, num, 0, PAGE_KERNEL);

  drm_gem_cma_create():
        cma_obj->vaddr = dma_alloc_wc(drm->dev, size, &cma_obj->paddr,
                                      GFP_KERNEL | __GFP_NOWARN);


- Could we set this range to the actual width/height of the native framebuffer?

  sdrm_drm_modeset_init():
	ddev->mode_config.min_width = 1;
	ddev->mode_config.min_height = 1;
	ddev->mode_config.max_width = 8192;
	ddev->mode_config.max_height = 8192;


- Is there a usecase for offsetting the dumb buffer when blitted onto the
  native buffer?

  sdrm_blit():
	/* get scanout offsets */
	xoff = 0;
	yoff = 0;
	if (sdrm->pipe.plane.fb == fb) {
		xoff = sdrm->pipe.crtc.x;
		yoff = sdrm->pipe.crtc.y;
	}


I have tested simpledrm on a Raspberry Pi B+ with U-boot setting up the
framebuffer and producing this node:

        framebuffer@1e887000 {
                compatible = "simple-framebuffer";
                reg = <0x1e887000 0x36c600>;
                format = "r5g6b5";
                width = <1824>;
                height = <984>;
                stride = <3648>;
                status = "okay";
        };

I have only tested with fbcon and modetest (XR24,RG16).


Noralf.


Changes from previous version[2]:
- Remove FB_SIMPLE=n dependency to avoid kconfig recursive error
- Changed module name to match kconfig help text: sdrm -> simpledrm
- Use drm_simple_display_pipe
- Replace deprecated drm_platform_init()
- sdrm_dumb_create(): drm_gem_object_unreference() -> *_unlocked()
- sdrm_dumb_map_offset(): drm_gem_object_lookup() remove drm_device parameter
- sdrm_drm_mmap() changes:
  Remove struct_mutex locking
  Add drm_vma_offset_{lock,unlock}_lookup()
  drm_mmap() -> drm_legacy_mmap()
- dma_buf_begin_cpu_access() doesn't require start and length anymore
- Use drm_cvt_mode() instead of open coding a mode
- Fix format conversion. In the intermediate step, store the 8/6/5 bit color
  value in the upper part of the 16-bit color variable, not the lower.
- Support clips == NULL in sdrm_dirty()
- Set mode_config.preferred_depth
- Attach mode_config.dirty_info_property to connector
fbdev:
- Remove the DRM_SIMPLEDRM_FBDEV kconfig option and use DRM_FBDEV_EMULATION
- Suspend fbcon/fbdev when the pipeline is enabled, resume in lastclose
- Add FBINFO_CAN_FORCE_OUTPUT flag so we get oops'es on the console

[1] https://lists.freedesktop.org/archives/dri-devel/2014-January/052584.html
[2] https://lists.freedesktop.org/archives/dri-devel/2014-January/052594.html


Further history:

[PATCH v4 0/6] SimpleDRM Driver
https://lists.freedesktop.org/archives/dri-devel/2013-September/044638.html

[PATCH v2 00/14] Platform Framebuffers and SimpleDRM
https://lists.freedesktop.org/archives/dri-devel/2013-July/041090.html

[RFC 0/6] SimpleDRM Driver (was: dvbe driver)
https://lists.freedesktop.org/archives/dri-devel/2013-June/040386.html

[PATCH 0/9] System Framebuffer Bus (sysfb)
https://lists.freedesktop.org/archives/dri-devel/2013-February/035013.html


Noralf Trønnes (2):
  drm: add SimpleDRM driver
  drm: simpledrm: add fbdev fallback support

 drivers/gpu/drm/Kconfig                      |   2 +
 drivers/gpu/drm/Makefile                     |   1 +
 drivers/gpu/drm/simpledrm/Kconfig            |  22 ++
 drivers/gpu/drm/simpledrm/Makefile           |   5 +
 drivers/gpu/drm/simpledrm/simpledrm.h        | 111 ++++++++++
 drivers/gpu/drm/simpledrm/simpledrm_damage.c | 314 +++++++++++++++++++++++++++
 drivers/gpu/drm/simpledrm/simpledrm_drv.c    | 298 +++++++++++++++++++++++++
 drivers/gpu/drm/simpledrm/simpledrm_fbdev.c  | 160 ++++++++++++++
 drivers/gpu/drm/simpledrm/simpledrm_gem.c    | 276 +++++++++++++++++++++++
 drivers/gpu/drm/simpledrm/simpledrm_kms.c    | 288 ++++++++++++++++++++++++
 10 files changed, 1477 insertions(+)
 create mode 100644 drivers/gpu/drm/simpledrm/Kconfig
 create mode 100644 drivers/gpu/drm/simpledrm/Makefile
 create mode 100644 drivers/gpu/drm/simpledrm/simpledrm.h
 create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_damage.c
 create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_drv.c
 create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
 create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_gem.c
 create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_kms.c

--
2.8.2

[PATCH 0/2] drm: add SimpleDRM driver

[Noralf Trønnes]

This patchset adds the simpledrm driver by David Herrmann based on a
patchset[1] from 2014. That patchset also included patches for kicking
out simpledrm by real drivers. I have stayed away from that since it
involves another subsystem and I would probably be unable to answer any
questions about the implementation.

I have done my best to bring simpledrm up to speed. However I was unable to
loose drm_legacy_mmap() in sdrm_drm_mmap() since I don't understand much of
the gem code.

I was left with some questions after doing this:

- Is there any reason why simpledrm can't use drm_gem_cma_helper?
  One obvious difference is that of contiguous memory allocation:

  sdrm_gem_get_pages():
	obj->pages = drm_malloc_ab(num, sizeof(*obj->pages));
	for (i = 0; i < num; ++i) {
		obj->pages[i] = alloc_page(GFP_KERNEL | __GFP_ZERO);
	obj->vmapping = vmap(obj->pages, num, 0, PAGE_KERNEL);

  drm_gem_cma_create():
        cma_obj->vaddr = dma_alloc_wc(drm->dev, size, &cma_obj->paddr,
                                      GFP_KERNEL | __GFP_NOWARN);


- Could we set this range to the actual width/height of the native framebuffer?

  sdrm_drm_modeset_init():
	ddev->mode_config.min_width = 1;
	ddev->mode_config.min_height = 1;
	ddev->mode_config.max_width = 8192;
	ddev->mode_config.max_height = 8192;


- Is there a usecase for offsetting the dumb buffer when blitted onto the
  native buffer?

  sdrm_blit():
	/* get scanout offsets */
	xoff = 0;
	yoff = 0;
	if (sdrm->pipe.plane.fb == fb) {
		xoff = sdrm->pipe.crtc.x;
		yoff = sdrm->pipe.crtc.y;
	}


I have tested simpledrm on a Raspberry Pi B+ with U-boot setting up the
framebuffer and producing this node:

        framebuffer@1e887000 {
                compatible = "simple-framebuffer";
                reg = <0x1e887000 0x36c600>;
                format = "r5g6b5";
                width = <1824>;
                height = <984>;
                stride = <3648>;
                status = "okay";
        };

I have only tested with fbcon and modetest (XR24,RG16).


Noralf.


Changes from previous version[2]:
- Remove FB_SIMPLE=n dependency to avoid kconfig recursive error
- Changed module name to match kconfig help text: sdrm -> simpledrm
- Use drm_simple_display_pipe
- Replace deprecated drm_platform_init()
- sdrm_dumb_create(): drm_gem_object_unreference() -> *_unlocked()
- sdrm_dumb_map_offset(): drm_gem_object_lookup() remove drm_device parameter
- sdrm_drm_mmap() changes:
  Remove struct_mutex locking
  Add drm_vma_offset_{lock,unlock}_lookup()
  drm_mmap() -> drm_legacy_mmap()
- dma_buf_begin_cpu_access() doesn't require start and length anymore
- Use drm_cvt_mode() instead of open coding a mode
- Fix format conversion. In the intermediate step, store the 8/6/5 bit color
  value in the upper part of the 16-bit color variable, not the lower.
- Support clips == NULL in sdrm_dirty()
- Set mode_config.preferred_depth
- Attach mode_config.dirty_info_property to connector
fbdev:
- Remove the DRM_SIMPLEDRM_FBDEV kconfig option and use DRM_FBDEV_EMULATION
- Suspend fbcon/fbdev when the pipeline is enabled, resume in lastclose
- Add FBINFO_CAN_FORCE_OUTPUT flag so we get oops'es on the console

[1] https://lists.freedesktop.org/archives/dri-devel/2014-January/052584.html
[2] https://lists.freedesktop.org/archives/dri-devel/2014-January/052594.html


Further history:

[PATCH v4 0/6] SimpleDRM Driver
https://lists.freedesktop.org/archives/dri-devel/2013-September/044638.html

[PATCH v2 00/14] Platform Framebuffers and SimpleDRM
https://lists.freedesktop.org/archives/dri-devel/2013-July/041090.html

[RFC 0/6] SimpleDRM Driver (was: dvbe driver)
https://lists.freedesktop.org/archives/dri-devel/2013-June/040386.html

[PATCH 0/9] System Framebuffer Bus (sysfb)
https://lists.freedesktop.org/archives/dri-devel/2013-February/035013.html


Noralf Trønnes (2):
  drm: add SimpleDRM driver
  drm: simpledrm: add fbdev fallback support

 drivers/gpu/drm/Kconfig                      |   2 +
 drivers/gpu/drm/Makefile                     |   1 +
 drivers/gpu/drm/simpledrm/Kconfig            |  22 ++
 drivers/gpu/drm/simpledrm/Makefile           |   5 +
 drivers/gpu/drm/simpledrm/simpledrm.h        | 111 ++++++++++
 drivers/gpu/drm/simpledrm/simpledrm_damage.c | 314 +++++++++++++++++++++++++++
 drivers/gpu/drm/simpledrm/simpledrm_drv.c    | 298 +++++++++++++++++++++++++
 drivers/gpu/drm/simpledrm/simpledrm_fbdev.c  | 160 ++++++++++++++
 drivers/gpu/drm/simpledrm/simpledrm_gem.c    | 276 +++++++++++++++++++++++
 drivers/gpu/drm/simpledrm/simpledrm_kms.c    | 288 ++++++++++++++++++++++++
 10 files changed, 1477 insertions(+)
 create mode 100644 drivers/gpu/drm/simpledrm/Kconfig
 create mode 100644 drivers/gpu/drm/simpledrm/Makefile
 create mode 100644 drivers/gpu/drm/simpledrm/simpledrm.h
 create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_damage.c
 create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_drv.c
 create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
 create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_gem.c
 create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_kms.c

--
2.8.2

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

Re: [PATCH 0/2] drm: add SimpleDRM driver

[Luc Verhaegen]

On Thu, Aug 04, 2016 at 04:03:18PM +0200, Noralf Trønnes wrote:
> 
> I have tested simpledrm on a Raspberry Pi B+ with U-boot setting up the
> framebuffer and producing this node:
> 
>         framebuffer@1e887000 {
>                 compatible = "simple-framebuffer";
>                 reg = <0x1e887000 0x36c600>;
>                 format = "r5g6b5";
>                 width = <1824>;
>                 height = <984>;
>                 stride = <3648>;
>                 status = "okay";
>         };
> 
> I have only tested with fbcon and modetest (XR24,RG16).

Please do not make the same mistake as simplefb in making this purely a 
rpifb. Know that you will need some power and clock management for 
properly free devices that do not depend on a binary only RTOS which 
does _everything_ behind our backs.

Cfr this useless, endless and ridiculous discussion: 
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/279071.html

Luc Verhaegen.

Re: [PATCH 0/2] drm: add SimpleDRM driver

[Luc Verhaegen]

On Thu, Aug 04, 2016 at 04:03:18PM +0200, Noralf Trønnes wrote:
> 
> I have tested simpledrm on a Raspberry Pi B+ with U-boot setting up the
> framebuffer and producing this node:
> 
>         framebuffer@1e887000 {
>                 compatible = "simple-framebuffer";
>                 reg = <0x1e887000 0x36c600>;
>                 format = "r5g6b5";
>                 width = <1824>;
>                 height = <984>;
>                 stride = <3648>;
>                 status = "okay";
>         };
> 
> I have only tested with fbcon and modetest (XR24,RG16).

Please do not make the same mistake as simplefb in making this purely a 
rpifb. Know that you will need some power and clock management for 
properly free devices that do not depend on a binary only RTOS which 
does _everything_ behind our backs.

Cfr this useless, endless and ridiculous discussion: 
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/279071.html

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

Re: [PATCH 0/2] drm: add SimpleDRM driver

[Daniel Vetter]

On Thu, Aug 04, 2016 at 04:15:25PM +0200, Luc Verhaegen wrote:
> On Thu, Aug 04, 2016 at 04:03:18PM +0200, Noralf Trønnes wrote:
> > 
> > I have tested simpledrm on a Raspberry Pi B+ with U-boot setting up the
> > framebuffer and producing this node:
> > 
> >         framebuffer@1e887000 {
> >                 compatible = "simple-framebuffer";
> >                 reg = <0x1e887000 0x36c600>;
> >                 format = "r5g6b5";
> >                 width = <1824>;
> >                 height = <984>;
> >                 stride = <3648>;
> >                 status = "okay";
> >         };
> > 
> > I have only tested with fbcon and modetest (XR24,RG16).
> 
> Please do not make the same mistake as simplefb in making this purely a 
> rpifb. Know that you will need some power and clock management for 
> properly free devices that do not depend on a binary only RTOS which 
> does _everything_ behind our backs.
> 
> Cfr this useless, endless and ridiculous discussion: 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/279071.html

simpledrm isn't a real driver, but only meant to be used to drive the
firmware framebuffer in early boot until a real driver takes over. It's a
replacement really for all the various uefi/vesa/whatever fbdev drivers.
Full reliance on the firmware very much intended.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 0/2] drm: add SimpleDRM driver

[Luc Verhaegen]

On Thu, Aug 04, 2016 at 05:08:43PM +0200, Daniel Vetter wrote:
> On Thu, Aug 04, 2016 at 04:15:25PM +0200, Luc Verhaegen wrote:
> > On Thu, Aug 04, 2016 at 04:03:18PM +0200, Noralf Trønnes wrote:
> > > 
> > > I have tested simpledrm on a Raspberry Pi B+ with U-boot setting up the
> > > framebuffer and producing this node:
> > > 
> > >         framebuffer@1e887000 {
> > >                 compatible = "simple-framebuffer";
> > >                 reg = <0x1e887000 0x36c600>;
> > >                 format = "r5g6b5";
> > >                 width = <1824>;
> > >                 height = <984>;
> > >                 stride = <3648>;
> > >                 status = "okay";
> > >         };
> > > 
> > > I have only tested with fbcon and modetest (XR24,RG16).
> > 
> > Please do not make the same mistake as simplefb in making this purely a 
> > rpifb. Know that you will need some power and clock management for 
> > properly free devices that do not depend on a binary only RTOS which 
> > does _everything_ behind our backs.
> > 
> > Cfr this useless, endless and ridiculous discussion: 
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/279071.html
> 
> simpledrm isn't a real driver, but only meant to be used to drive the
> firmware framebuffer in early boot until a real driver takes over. It's a
> replacement really for all the various uefi/vesa/whatever fbdev drivers.
> Full reliance on the firmware very much intended.
> -Daniel

In the sunxi case, that firmware was u-boot, and the clocks were 
properly declared and then also properly disabled, which meant the 
display block got disabled.

Is simpledrm only an intermediate solution until the real driver is 
loaded? What stops it from providing a rudimentary display driver for 
the whole uptime of the machine? What stops the kernel from disabling 
the clocks while the supposed real driver is not fully loaded yet?

Do we really want to recreate a 400+ email thread again, or are we 
capable of learning from the past?

Luc Verhaegen.

Re: [PATCH 0/2] drm: add SimpleDRM driver

[David Herrmann]

Hi

On Thu, Aug 4, 2016 at 5:34 PM, Luc Verhaegen <libv@skynet.be> wrote:
> Do we really want to recreate a 400+ email thread again, or are we
> capable of learning from the past?

No we don't. And no-one intends to. I am fully aware of the discussion
that introduced the clock-dependencies to simplefb, and I gladly
accept patches that add such support to SimpleDRM. Did anyone say
otherwise? This series adds initial support for the devices _we_ know
and can test (which is x86 and RPi, in my case). If someone wants
support for more devices, please send patches. Why does this have to
be included (or even discussed) as part of this submission?

Thanks
David

Re: [PATCH 0/2] drm: add SimpleDRM driver

[David Herrmann]

Hi

On Thu, Aug 4, 2016 at 5:34 PM, Luc Verhaegen <libv@skynet.be> wrote:
> Do we really want to recreate a 400+ email thread again, or are we
> capable of learning from the past?

No we don't. And no-one intends to. I am fully aware of the discussion
that introduced the clock-dependencies to simplefb, and I gladly
accept patches that add such support to SimpleDRM. Did anyone say
otherwise? This series adds initial support for the devices _we_ know
and can test (which is x86 and RPi, in my case). If someone wants
support for more devices, please send patches. Why does this have to
be included (or even discussed) as part of this submission?

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

Re: [PATCH 0/2] drm: add SimpleDRM driver

[Luc Verhaegen]

On Thu, Aug 04, 2016 at 05:44:23PM +0200, David Herrmann wrote:
> Hi
> 
> On Thu, Aug 4, 2016 at 5:34 PM, Luc Verhaegen <libv@skynet.be> wrote:
> > Do we really want to recreate a 400+ email thread again, or are we
> > capable of learning from the past?
> 
> No we don't. And no-one intends to. I am fully aware of the discussion
> that introduced the clock-dependencies to simplefb, and I gladly
> accept patches that add such support to SimpleDRM. Did anyone say
> otherwise? This series adds initial support for the devices _we_ know
> and can test (which is x86 and RPi, in my case). If someone wants
> support for more devices, please send patches. Why does this have to
> be included (or even discussed) as part of this submission?

You're right, it does not have to be included until there is a usecase.

But on the otherhand i have become pretty allergic to this as that other 
discussion was completely useless and pointless and stemmed only from 
the fact that people had been kidding themselves all along that simplefb 
was going to be simple and not would need anything extra.

If we can avoid fooling ourselves to the same extent as we did before, 
then putting this off until there is a real usecase is perfectly fine by 
me.

If not, just add this trivial generic addition straight from an existing 
example. 

Luc Verhaegen.

Re: [PATCH 0/2] drm: add SimpleDRM driver

[Daniel Vetter]

On Thu, Aug 04, 2016 at 05:59:42PM +0200, Luc Verhaegen wrote:
> On Thu, Aug 04, 2016 at 05:44:23PM +0200, David Herrmann wrote:
> > On Thu, Aug 4, 2016 at 5:34 PM, Luc Verhaegen <libv@skynet.be> wrote:
> > > Do we really want to recreate a 400+ email thread again, or are we
> > > capable of learning from the past?
> > 
> > No we don't. And no-one intends to. I am fully aware of the discussion
> > that introduced the clock-dependencies to simplefb, and I gladly
> > accept patches that add such support to SimpleDRM. Did anyone say
> > otherwise? This series adds initial support for the devices _we_ know
> > and can test (which is x86 and RPi, in my case). If someone wants
> > support for more devices, please send patches. Why does this have to
> > be included (or even discussed) as part of this submission?
> 
> You're right, it does not have to be included until there is a usecase.
> 
> But on the otherhand i have become pretty allergic to this as that other 
> discussion was completely useless and pointless and stemmed only from 
> the fact that people had been kidding themselves all along that simplefb 
> was going to be simple and not would need anything extra.
> 
> If we can avoid fooling ourselves to the same extent as we did before, 
> then putting this off until there is a real usecase is perfectly fine by 
> me.
> 
> If not, just add this trivial generic addition straight from an existing 
> example. 

I stand corrected on this, seems indeed we need this to avoid issues. I
guess on x86 we don't need this since a pci device is a mostly
well-defined entity, and as long as we stop vesa/uefi for simpledrm before
the real driver loads it'll be all fine.

But on arm it's indeed different with clocks (and I guess sooner or later
some power domains too, or whatever else really). I think as long as it
doesn't go beyond "grab resource on load, drop it again on unload" I'm
prefectly fine with adding all that's needed to simpledrmfb. And totally
up to the folks enabling this whether it should be there upfront or only
once needed for a given platform.
-Daniel

> 
> Luc Verhaegen.
> _______________________________________________
> 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

Re: [PATCH 0/2] drm: add SimpleDRM driver

[One Thousand Gnomes]

> firmware framebuffer in early boot until a real driver takes over. It's a
> replacement really for all the various uefi/vesa/whatever fbdev drivers.
> Full reliance on the firmware very much intended.

Most of those have firmware interfaces for things like colour setting and
hardware scrolling. It's a replacement for far less than might first
appear but still useful.

Alan

Re: [PATCH 0/2] drm: add SimpleDRM driver

[One Thousand Gnomes]

> firmware framebuffer in early boot until a real driver takes over. It's a
> replacement really for all the various uefi/vesa/whatever fbdev drivers.
> Full reliance on the firmware very much intended.

Most of those have firmware interfaces for things like colour setting and
hardware scrolling. It's a replacement for far less than might first
appear but still useful.

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

Re: [PATCH 0/2] drm: add SimpleDRM driver

[Noralf Trønnes]

Den 04.08.2016 16:15, skrev Luc Verhaegen:
> On Thu, Aug 04, 2016 at 04:03:18PM +0200, Noralf Trønnes wrote:
>> I have tested simpledrm on a Raspberry Pi B+ with U-boot setting up the
>> framebuffer and producing this node:
>>
>>          framebuffer@1e887000 {
>>                  compatible = "simple-framebuffer";
>>                  reg = <0x1e887000 0x36c600>;
>>                  format = "r5g6b5";
>>                  width = <1824>;
>>                  height = <984>;
>>                  stride = <3648>;
>>                  status = "okay";
>>          };
>>
>> I have only tested with fbcon and modetest (XR24,RG16).
> Please do not make the same mistake as simplefb in making this purely a
> rpifb. Know that you will need some power and clock management for
> properly free devices that do not depend on a binary only RTOS which
> does _everything_ behind our backs.

I didn't read the binding document[1], which I should have done.
If simpledrm claims to be compatible with simple-framebuffer I assume it
should support the entire binding doc which includes clocks, regulators
and having the node under /chosen.
I will lift the necessary code from simplefb.c and put it in the next
version.

The binding doc also mentions an optional display phandle property, but I
can't find any reference to this in simplefb.c.


Noralf.

[1] Documentation/devicetree/bindings/display/simple-framebuffer.txt

Re: [PATCH 0/2] drm: add SimpleDRM driver

[Noralf Trønnes]

Den 04.08.2016 16:15, skrev Luc Verhaegen:
> On Thu, Aug 04, 2016 at 04:03:18PM +0200, Noralf Trønnes wrote:
>> I have tested simpledrm on a Raspberry Pi B+ with U-boot setting up the
>> framebuffer and producing this node:
>>
>>          framebuffer@1e887000 {
>>                  compatible = "simple-framebuffer";
>>                  reg = <0x1e887000 0x36c600>;
>>                  format = "r5g6b5";
>>                  width = <1824>;
>>                  height = <984>;
>>                  stride = <3648>;
>>                  status = "okay";
>>          };
>>
>> I have only tested with fbcon and modetest (XR24,RG16).
> Please do not make the same mistake as simplefb in making this purely a
> rpifb. Know that you will need some power and clock management for
> properly free devices that do not depend on a binary only RTOS which
> does _everything_ behind our backs.

I didn't read the binding document[1], which I should have done.
If simpledrm claims to be compatible with simple-framebuffer I assume it
should support the entire binding doc which includes clocks, regulators
and having the node under /chosen.
I will lift the necessary code from simplefb.c and put it in the next
version.

The binding doc also mentions an optional display phandle property, but I
can't find any reference to this in simplefb.c.


Noralf.

[1] Documentation/devicetree/bindings/display/simple-framebuffer.txt

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

Re: [PATCH 0/2] drm: add SimpleDRM driver

[Luc Verhaegen]

On Thu, Aug 04, 2016 at 06:58:55PM +0200, Noralf Trønnes wrote:
> 
> I didn't read the binding document[1], which I should have done.
> If simpledrm claims to be compatible with simple-framebuffer I assume it
> should support the entire binding doc which includes clocks, regulators
> and having the node under /chosen.
> I will lift the necessary code from simplefb.c and put it in the next
> version.

Smashing, repeat of a massive pain avoided, thanks :)

> The binding doc also mentions an optional display phandle property, but I
> can't find any reference to this in simplefb.c.

By that time i had long, long given up on this, i think Hans de Goede, 
who is a few orders of magnitude more patient than me, should know. He 
valiantly whipped this thing through back then.

Luc Verhaegen.

Re: [PATCH 0/2] drm: add SimpleDRM driver

[Hans de Goede]

Hi,

On 04-08-16 20:12, Luc Verhaegen wrote:
> On Thu, Aug 04, 2016 at 06:58:55PM +0200, Noralf Trønnes wrote:
>>
>> I didn't read the binding document[1], which I should have done.
>> If simpledrm claims to be compatible with simple-framebuffer I assume it
>> should support the entire binding doc which includes clocks, regulators
>> and having the node under /chosen.
>> I will lift the necessary code from simplefb.c and put it in the next
>> version.
>
> Smashing, repeat of a massive pain avoided, thanks :)
>
>> The binding doc also mentions an optional display phandle property, but I
>> can't find any reference to this in simplefb.c.

Ah yes, the display phandle, so the idea behind this is that the
simplefb node would have a display phandle pointing to a node
describing the "primary" node describing the actual display-pipe hardware.

The primary language is there because a display pipeline typically
consists of multiple blocks and thus has multiple nodes describing it.

This way the hardware driver would be able to figure out which simplefb
to disable if there is more then 1.

In practice the remove_conflicting_framebuffers kernel API is used for this and
that takes a framebuffer address, so that bit of the bindings is essentially
unused. Either way that bit is only relevant to the actual display hardware driver
(so that it can disable sumplefb when it takes over the display) and for
simpledrm you can simply ignore it.

Regards,

Hans

p.s.

Noralf, I recognize your name from the ft6236 touchscreen driver, I've mailed
you about this in the past because it is a duplicate driver, the edt-ft5x06
driver already speaks the same protocol. I see now that I made a copy and paste
error in your email address, so you never got my mails on this. I'll resend
my latest mail (a kernel patch removing the duplicate driver!) with a fixed
email address.

Re: [PATCH 0/2] drm: add SimpleDRM driver

[Daniel Vetter]

On Thu, Aug 04, 2016 at 04:03:18PM +0200, Noralf Trønnes wrote:
> This patchset adds the simpledrm driver by David Herrmann based on a
> patchset[1] from 2014. That patchset also included patches for kicking
> out simpledrm by real drivers. I have stayed away from that since it
> involves another subsystem and I would probably be unable to answer any
> questions about the implementation.

Need David's input on this, but I think the force-removal of simpledrm
when other drivers take over is required. Otherwise hilarity can ensue.
If we leave this out for the inital merge then I think we need a really
big warning in Kconfig that if people aren't careful it could result in a
kaboom.

> I have done my best to bring simpledrm up to speed. However I was unable to
> loose drm_legacy_mmap() in sdrm_drm_mmap() since I don't understand much of
> the gem code.

You can just nuke drm_legacy_mmap and the if check, plus the drm_legacy.h
include. There should be no reason at all for that.

> I was left with some questions after doing this:
> 
> - Is there any reason why simpledrm can't use drm_gem_cma_helper?
>   One obvious difference is that of contiguous memory allocation:
> 
>   sdrm_gem_get_pages():
> 	obj->pages = drm_malloc_ab(num, sizeof(*obj->pages));
> 	for (i = 0; i < num; ++i) {
> 		obj->pages[i] = alloc_page(GFP_KERNEL | __GFP_ZERO);
> 	obj->vmapping = vmap(obj->pages, num, 0, PAGE_KERNEL);
> 
>   drm_gem_cma_create():
>         cma_obj->vaddr = dma_alloc_wc(drm->dev, size, &cma_obj->paddr,
>                                       GFP_KERNEL | __GFP_NOWARN);

cma means contiguous memory allocator, that's pretty much the entire
point. Once you have that there's not much left really. Note that since
sdrm was submitted we gained some helpers for normal (shmem-backed) gem
buffers. I think we could extend those a bit to provide the basics for
dumb gem based buffers and use that in sdrm. Otoh there's probably only
udl which could benefit, so not sure this is worth it much. And mmap
helper, maybe.

> - Could we set this range to the actual width/height of the native framebuffer?
> 
>   sdrm_drm_modeset_init():
> 	ddev->mode_config.min_width = 1;
> 	ddev->mode_config.min_height = 1;
> 	ddev->mode_config.max_width = 8192;
> 	ddev->mode_config.max_height = 8192;

Probably a good idea if we don't support modeset changes with simpledrm.

> - Is there a usecase for offsetting the dumb buffer when blitted onto the
>   native buffer?
> 
>   sdrm_blit():
> 	/* get scanout offsets */
> 	xoff = 0;
> 	yoff = 0;
> 	if (sdrm->pipe.plane.fb == fb) {
> 		xoff = sdrm->pipe.crtc.x;
> 		yoff = sdrm->pipe.crtc.y;
> 	}

I don't think allowing the plane to be positioned is a good idea. And
since it's using the simple pipe helpers now that should be even
impossible.

> I have tested simpledrm on a Raspberry Pi B+ with U-boot setting up the
> framebuffer and producing this node:
> 
>         framebuffer@1e887000 {
>                 compatible = "simple-framebuffer";
>                 reg = <0x1e887000 0x36c600>;
>                 format = "r5g6b5";
>                 width = <1824>;
>                 height = <984>;
>                 stride = <3648>;
>                 status = "okay";
>         };
> 
> I have only tested with fbcon and modetest (XR24,RG16).

If you can, booting this on a vesa/uefi platform would be interesting too,
just to make sure.
-Daniel


> 
> 
> Noralf.
> 
> 
> Changes from previous version[2]:
> - Remove FB_SIMPLE=n dependency to avoid kconfig recursive error
> - Changed module name to match kconfig help text: sdrm -> simpledrm
> - Use drm_simple_display_pipe
> - Replace deprecated drm_platform_init()
> - sdrm_dumb_create(): drm_gem_object_unreference() -> *_unlocked()
> - sdrm_dumb_map_offset(): drm_gem_object_lookup() remove drm_device parameter
> - sdrm_drm_mmap() changes:
>   Remove struct_mutex locking
>   Add drm_vma_offset_{lock,unlock}_lookup()
>   drm_mmap() -> drm_legacy_mmap()
> - dma_buf_begin_cpu_access() doesn't require start and length anymore
> - Use drm_cvt_mode() instead of open coding a mode
> - Fix format conversion. In the intermediate step, store the 8/6/5 bit color
>   value in the upper part of the 16-bit color variable, not the lower.
> - Support clips == NULL in sdrm_dirty()
> - Set mode_config.preferred_depth
> - Attach mode_config.dirty_info_property to connector
> fbdev:
> - Remove the DRM_SIMPLEDRM_FBDEV kconfig option and use DRM_FBDEV_EMULATION
> - Suspend fbcon/fbdev when the pipeline is enabled, resume in lastclose
> - Add FBINFO_CAN_FORCE_OUTPUT flag so we get oops'es on the console
> 
> [1] https://lists.freedesktop.org/archives/dri-devel/2014-January/052584.html
> [2] https://lists.freedesktop.org/archives/dri-devel/2014-January/052594.html
> 
> 
> Further history:
> 
> [PATCH v4 0/6] SimpleDRM Driver
> https://lists.freedesktop.org/archives/dri-devel/2013-September/044638.html
> 
> [PATCH v2 00/14] Platform Framebuffers and SimpleDRM
> https://lists.freedesktop.org/archives/dri-devel/2013-July/041090.html
> 
> [RFC 0/6] SimpleDRM Driver (was: dvbe driver)
> https://lists.freedesktop.org/archives/dri-devel/2013-June/040386.html
> 
> [PATCH 0/9] System Framebuffer Bus (sysfb)
> https://lists.freedesktop.org/archives/dri-devel/2013-February/035013.html
> 
> 
> Noralf Trønnes (2):
>   drm: add SimpleDRM driver
>   drm: simpledrm: add fbdev fallback support
> 
>  drivers/gpu/drm/Kconfig                      |   2 +
>  drivers/gpu/drm/Makefile                     |   1 +
>  drivers/gpu/drm/simpledrm/Kconfig            |  22 ++
>  drivers/gpu/drm/simpledrm/Makefile           |   5 +
>  drivers/gpu/drm/simpledrm/simpledrm.h        | 111 ++++++++++
>  drivers/gpu/drm/simpledrm/simpledrm_damage.c | 314 +++++++++++++++++++++++++++
>  drivers/gpu/drm/simpledrm/simpledrm_drv.c    | 298 +++++++++++++++++++++++++
>  drivers/gpu/drm/simpledrm/simpledrm_fbdev.c  | 160 ++++++++++++++
>  drivers/gpu/drm/simpledrm/simpledrm_gem.c    | 276 +++++++++++++++++++++++
>  drivers/gpu/drm/simpledrm/simpledrm_kms.c    | 288 ++++++++++++++++++++++++
>  10 files changed, 1477 insertions(+)
>  create mode 100644 drivers/gpu/drm/simpledrm/Kconfig
>  create mode 100644 drivers/gpu/drm/simpledrm/Makefile
>  create mode 100644 drivers/gpu/drm/simpledrm/simpledrm.h
>  create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_damage.c
>  create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_drv.c
>  create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
>  create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_gem.c
>  create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_kms.c
> 
> --
> 2.8.2
> 
> _______________________________________________
> 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

Re: [PATCH 0/2] drm: add SimpleDRM driver

[Noralf Trønnes]

Den 04.08.2016 16:36, skrev Daniel Vetter:
> On Thu, Aug 04, 2016 at 04:03:18PM +0200, Noralf Trønnes wrote:
>> This patchset adds the simpledrm driver by David Herrmann based on a
>> patchset[1] from 2014. That patchset also included patches for kicking
>> out simpledrm by real drivers. I have stayed away from that since it
>> involves another subsystem and I would probably be unable to answer any
>> questions about the implementation.
> Need David's input on this, but I think the force-removal of simpledrm
> when other drivers take over is required. Otherwise hilarity can ensue.
> If we leave this out for the inital merge then I think we need a really
> big warning in Kconfig that if people aren't careful it could result in a
> kaboom.

I understand. I anticipated this, but I was hoping that if I moved this 
part forward,
which I was capable of doing, maybe someone else could do the rest :-)
Anyways, if no one chimes in to do this, lets add a warning.

> If you can, booting this on a vesa/uefi platform would be interesting too,
> just to make sure.

I'm a bit limited here I'm afraid. I only have a Raspberry Pi to test with.
My other Linux installations are running on a VMware ESXi server.


Noralf.