Re: [PATCH] drm/cirrus: rewrite and modernize driver.
Hi, > So I guess I have to add a dest_clip bool parameter when moving them. > /me looks for a good place. drm_fb_helpers.c I think. https://git.kraxel.org/cgit/linux/log/?h=drm-rewrite-cirrus updated. v2 series will follow later today or tomorrow. cheers, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/cirrus: rewrite and modernize driver.
Den 04.04.2019 12.27, skrev Gerd Hoffmann: > Hi, > >>> tinydrm_xrgb_to_* >>> >>> imo these could be put into some drm_format_helpers.c to be shared. >> >> I agree, my long term goal is to get rid of tinydrm.ko. Just haven't got >> there yet. >> >> Gerd, if you end up using some of those functions, feel free to move >> just those you need and I can do the rest later. But if you have time to >> spare I wouldn't mind getting all of them moved ;-) > > For now I just promoted cirrus to be a tinydrm driver ;) > > Noticed that those helpers (including tinydrm_memcpy for the > non-converting case) apply clipping on the source but not on > the destination. So, for fullscreen updates that works ok, > but for updating sub-rectangles it doesn't ... Ah yes, these MIPI type controllers support setting the destination window in controller RAM for the incoming buffer so there has been no need for clipping on the destination buffer. > > So I guess I have to add a dest_clip bool parameter when moving them. > /me looks for a good place. drm_fb_helpers.c I think. > > cheers, > Gerd > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/cirrus: rewrite and modernize driver.
Hi, > > tinydrm_xrgb_to_* > > > > imo these could be put into some drm_format_helpers.c to be shared. > > I agree, my long term goal is to get rid of tinydrm.ko. Just haven't got > there yet. > > Gerd, if you end up using some of those functions, feel free to move > just those you need and I can do the rest later. But if you have time to > spare I wouldn't mind getting all of them moved ;-) For now I just promoted cirrus to be a tinydrm driver ;) Noticed that those helpers (including tinydrm_memcpy for the non-converting case) apply clipping on the source but not on the destination. So, for fullscreen updates that works ok, but for updating sub-rectangles it doesn't ... So I guess I have to add a dest_clip bool parameter when moving them. /me looks for a good place. drm_fb_helpers.c I think. cheers, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/cirrus: rewrite and modernize driver.
Den 04.04.2019 10.52, skrev Daniel Vetter: > On Thu, Apr 4, 2019 at 10:30 AM Gerd Hoffmann wrote: >> >> Hi, >> Speaking of wayland: Seems at least gnome-shell insists on using XR24. >>> >>> Yeah XR24 is pretty much mandatory. Noralf added a few helpers to >>> convert XR24 to other formats, for display not supporting anything >>> else. Because userspace. >> >> Have a pointer to these helpers? grepping around in drm didn't turn up >> anything so far ... > > tinydrm_xrgb_to_* > > imo these could be put into some drm_format_helpers.c to be shared. I agree, my long term goal is to get rid of tinydrm.ko. Just haven't got there yet. Gerd, if you end up using some of those functions, feel free to move just those you need and I can do the rest later. But if you have time to spare I wouldn't mind getting all of them moved ;-) Noralf. > From a quick look the xrgb_to_rgb888 is missing, but for a quick > hack you can just use rgb565 to get going. > -Daniel > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/cirrus: rewrite and modernize driver.
On Thu, Apr 4, 2019 at 10:30 AM Gerd Hoffmann wrote: > > Hi, > > > > Speaking of wayland: Seems at least gnome-shell insists on using XR24. > > > > Yeah XR24 is pretty much mandatory. Noralf added a few helpers to > > convert XR24 to other formats, for display not supporting anything > > else. Because userspace. > > Have a pointer to these helpers? grepping around in drm didn't turn up > anything so far ... tinydrm_xrgb_to_* imo these could be put into some drm_format_helpers.c to be shared. From a quick look the xrgb_to_rgb888 is missing, but for a quick hack you can just use rgb565 to get going. -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] drm/cirrus: rewrite and modernize driver.
Hi, > > Speaking of wayland: Seems at least gnome-shell insists on using XR24. > > Yeah XR24 is pretty much mandatory. Noralf added a few helpers to > convert XR24 to other formats, for display not supporting anything > else. Because userspace. Have a pointer to these helpers? grepping around in drm didn't turn up anything so far ... thanks, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/cirrus: rewrite and modernize driver.
Hi, > > Speaking of wayland: Seems at least gnome-shell insists on using XR24. > > Yeah XR24 is pretty much mandatory. Noralf added a few helpers to > convert XR24 to other formats, for display not supporting anything > else. Because userspace. Ah, right, that is an option too. Given we blit any display updates anyway we could easily convert XR24 to RG24 while doing so. Guess that is better ... > > Well, I can reintroduce the module option. I don't see any other > > reasonable way to support 32bpp. If the driver reports XR24 as > > supported and also adds the higher resolutions (which work with RG16 > > only) to the mode list userspace will of course try the higher > > resolutions with XR24 and struggle ... > > Maybe atomic userspace is better (it should be, it can do TEST_ONLY), > but I'm not so sure that exposing all modes for atomic clients would > work. Also, currently not possible with our probe helpers (we don't > refilter the list per client). .. than this mess. cheers, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/cirrus: rewrite and modernize driver.
On Thu, Apr 4, 2019 at 7:51 AM Gerd Hoffmann wrote: > > On Thu, Apr 04, 2019 at 12:58:09PM +1000, David Airlie wrote: > > On Wed, Apr 3, 2019 at 5:23 PM Gerd Hoffmann wrote: > > > > > > Time to kill some bad sample code people are copying from ;) > > > > > > This is a complete rewrite of the cirrus driver. The cirrus_mode_set() > > > function is pretty much the only function which is carried over largely > > > unmodified. Everything else is upside down. > > > > > > It is a single monster patch. But given that it does some pretty > > > fundamental changes to the drivers workflow and also reduces the code > > > size by roughly 70% I think it'll still be alot easier to review than a > > > longish baby-step patch series. > > > > > > Changes summary: > > > - Given the small amout of video memory (4 MB) the cirrus device has > > >the rewritten driver doesn't try to manage buffers there. Instead > > >it will blit (memcpy) the active framebuffer to video memory. > > > > Does it get any slower, with TTM I just wrote it to migrate just the > > frontbuffer in/out of VRAM on modeset, won't we end up with more > > copies now? > > I didn't benchmark it, but if you care about performance you should not > be using cirrus anyway ... > > For fbcon it probably doesn't make much of a difference, fbcon used a > shadow framebuffer before (for fbdev mmap and dirty tracking). > > xorg probably ends up with more copying. > > Anything doing display updates with page flips (i.e wayland) should end > up with less copying, because you have one copy (blit) instead of two > copies (migrate old frontbuffer out of vram, migrate new frontbuffer > into vram) on pageflip. > > Speaking of wayland: Seems at least gnome-shell insists on using XR24. Yeah XR24 is pretty much mandatory. Noralf added a few helpers to convert XR24 to other formats, for display not supporting anything else. Because userspace. > > > - Only DRM_FORMAT_RGB565 (depth 16) is supported. The old driver does > > >that too by default. There was a module parameter which enables 24/32 > > >bpp support and disables higher resolutions (due to cirrus hardware > > >constrains). That parameter wasn't reimplemented. > > > This might be the big sticking point, this is a userspace regression > > for a feature that was explicitly added a few years ago, can we really > > get away without it? > > Well, I can reintroduce the module option. I don't see any other > reasonable way to support 32bpp. If the driver reports XR24 as > supported and also adds the higher resolutions (which work with RG16 > only) to the mode list userspace will of course try the higher > resolutions with XR24 and struggle ... Maybe atomic userspace is better (it should be, it can do TEST_ONLY), but I'm not so sure that exposing all modes for atomic clients would work. Also, currently not possible with our probe helpers (we don't refilter the list per client). -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] drm/cirrus: rewrite and modernize driver.
On Thu, Apr 04, 2019 at 12:58:09PM +1000, David Airlie wrote: > On Wed, Apr 3, 2019 at 5:23 PM Gerd Hoffmann wrote: > > > > Time to kill some bad sample code people are copying from ;) > > > > This is a complete rewrite of the cirrus driver. The cirrus_mode_set() > > function is pretty much the only function which is carried over largely > > unmodified. Everything else is upside down. > > > > It is a single monster patch. But given that it does some pretty > > fundamental changes to the drivers workflow and also reduces the code > > size by roughly 70% I think it'll still be alot easier to review than a > > longish baby-step patch series. > > > > Changes summary: > > - Given the small amout of video memory (4 MB) the cirrus device has > >the rewritten driver doesn't try to manage buffers there. Instead > >it will blit (memcpy) the active framebuffer to video memory. > > Does it get any slower, with TTM I just wrote it to migrate just the > frontbuffer in/out of VRAM on modeset, won't we end up with more > copies now? I didn't benchmark it, but if you care about performance you should not be using cirrus anyway ... For fbcon it probably doesn't make much of a difference, fbcon used a shadow framebuffer before (for fbdev mmap and dirty tracking). xorg probably ends up with more copying. Anything doing display updates with page flips (i.e wayland) should end up with less copying, because you have one copy (blit) instead of two copies (migrate old frontbuffer out of vram, migrate new frontbuffer into vram) on pageflip. Speaking of wayland: Seems at least gnome-shell insists on using XR24. > > - Only DRM_FORMAT_RGB565 (depth 16) is supported. The old driver does > >that too by default. There was a module parameter which enables 24/32 > >bpp support and disables higher resolutions (due to cirrus hardware > >constrains). That parameter wasn't reimplemented. > This might be the big sticking point, this is a userspace regression > for a feature that was explicitly added a few years ago, can we really > get away without it? Well, I can reintroduce the module option. I don't see any other reasonable way to support 32bpp. If the driver reports XR24 as supported and also adds the higher resolutions (which work with RG16 only) to the mode list userspace will of course try the higher resolutions with XR24 and struggle ... cheers, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/cirrus: rewrite and modernize driver.
On Wed, Apr 3, 2019 at 7:58 PM David Airlie wrote: > On Wed, Apr 3, 2019 at 5:23 PM Gerd Hoffmann wrote: > > > > Time to kill some bad sample code people are copying from ;) > > > > This is a complete rewrite of the cirrus driver. The cirrus_mode_set() > > function is pretty much the only function which is carried over largely > > unmodified. Everything else is upside down. > > > > It is a single monster patch. But given that it does some pretty > > fundamental changes to the drivers workflow and also reduces the code > > size by roughly 70% I think it'll still be alot easier to review than a > > longish baby-step patch series. > > > > Changes summary: > > - Given the small amout of video memory (4 MB) the cirrus device has > >the rewritten driver doesn't try to manage buffers there. Instead > >it will blit (memcpy) the active framebuffer to video memory. > > Does it get any slower, with TTM I just wrote it to migrate just the > frontbuffer in/out of VRAM on modeset, won't we end up with more > copies now? > > > - All gem objects are stored in main memory and are manged using the > >new shmem helpers. ttm is out. > > - Only DRM_FORMAT_RGB565 (depth 16) is supported. The old driver does > >that too by default. There was a module parameter which enables 24/32 > >bpp support and disables higher resolutions (due to cirrus hardware > >constrains). That parameter wasn't reimplemented. > This might be the big sticking point, this is a userspace regression > for a feature that was explicitly added a few years ago, can we really > get away without it? > Chrome OS testing in VMs was one of the consumers of 32bpp on cirrus, and we have gotten rid of cirrus in favor of virtio gpu, so we'd be fine. Of course I can't speak for other consumers :) Stéphane > > The rest looks good though! > Dave. > > > - The simple display pipeline is used. > > - The generic fbdev emulation is used. > > - It's a atomic driver now. > > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/cirrus: rewrite and modernize driver.
On Wed, Apr 3, 2019 at 5:23 PM Gerd Hoffmann wrote: > > Time to kill some bad sample code people are copying from ;) > > This is a complete rewrite of the cirrus driver. The cirrus_mode_set() > function is pretty much the only function which is carried over largely > unmodified. Everything else is upside down. > > It is a single monster patch. But given that it does some pretty > fundamental changes to the drivers workflow and also reduces the code > size by roughly 70% I think it'll still be alot easier to review than a > longish baby-step patch series. > > Changes summary: > - Given the small amout of video memory (4 MB) the cirrus device has >the rewritten driver doesn't try to manage buffers there. Instead >it will blit (memcpy) the active framebuffer to video memory. Does it get any slower, with TTM I just wrote it to migrate just the frontbuffer in/out of VRAM on modeset, won't we end up with more copies now? > - All gem objects are stored in main memory and are manged using the >new shmem helpers. ttm is out. > - Only DRM_FORMAT_RGB565 (depth 16) is supported. The old driver does >that too by default. There was a module parameter which enables 24/32 >bpp support and disables higher resolutions (due to cirrus hardware >constrains). That parameter wasn't reimplemented. This might be the big sticking point, this is a userspace regression for a feature that was explicitly added a few years ago, can we really get away without it? The rest looks good though! Dave. > - The simple display pipeline is used. > - The generic fbdev emulation is used. > - It's a atomic driver now. > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/cirrus: rewrite and modernize driver.
On Wed, 2019-04-03 at 16:15 +0100, Daniel Stone wrote: > There's already a list of supported formats for each DRM plane, which > you can get via drmModeGetPlane (being careful to enable universal > planes so you can discover the primary plane). The same information is > present in the 'IN_FORMATS' property, which is more difficult to parse > but also tells you about modifiers. > > modesetting already pulls all this out (at least in the atomic path) > so we can reason about acceptable modifiers. D'oh, I knew that. The problem then is modesetting isn't pulling that info out early enough in PreInit. I'll consider that another case of xorg/xserver#9 then. - ajax ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/cirrus: rewrite and modernize driver.
On Wed, 3 Apr 2019 at 16:12, Adam Jackson wrote: > On Wed, 2019-04-03 at 09:23 +0200, Gerd Hoffmann wrote: > > - Only DRM_FORMAT_RGB565 (depth 16) is supported. The old driver does > >that too by default. There was a module parameter which enables 24/32 > >bpp support and disables higher resolutions (due to cirrus hardware > >constrains). That parameter wasn't reimplemented. > > One slightly annoying aspect of this (well, initially of the patch to > clamp the default to 16bpp, but this too) is that we only have a way to > ask the driver which format it prefers, not which ones it supports at > all. For X's modesetting driver (and yes some of this is because X is > awful) this creates the following failure mode: > > 1: user sets up xorg.conf for depth 24 > 2: user upgrades kernel, reboots > 3: X driver detects that depth 16 is preferred, but > 4: X core respects user's xorg.conf and tries depth 24, which > 5: throws -EINVAL and X won't start. > > Possibly X should work around this by transparently setting up a shadow > framebuffer at the user's requested depth. The problem there is, if 565 > is preferred but works, you're adding a format-conversion blit in > the middle for no reason. If I could ask the kernel for the entire list > of supported formats, I could only set up the shadow if it was > necessary. There's already a list of supported formats for each DRM plane, which you can get via drmModeGetPlane (being careful to enable universal planes so you can discover the primary plane). The same information is present in the 'IN_FORMATS' property, which is more difficult to parse but also tells you about modifiers. modesetting already pulls all this out (at least in the atomic path) so we can reason about acceptable modifiers. Cheers, Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/cirrus: rewrite and modernize driver.
On Wed, 2019-04-03 at 09:23 +0200, Gerd Hoffmann wrote: > - Only DRM_FORMAT_RGB565 (depth 16) is supported. The old driver does >that too by default. There was a module parameter which enables 24/32 >bpp support and disables higher resolutions (due to cirrus hardware >constrains). That parameter wasn't reimplemented. One slightly annoying aspect of this (well, initially of the patch to clamp the default to 16bpp, but this too) is that we only have a way to ask the driver which format it prefers, not which ones it supports at all. For X's modesetting driver (and yes some of this is because X is awful) this creates the following failure mode: 1: user sets up xorg.conf for depth 24 2: user upgrades kernel, reboots 3: X driver detects that depth 16 is preferred, but 4: X core respects user's xorg.conf and tries depth 24, which 5: throws -EINVAL and X won't start. Possibly X should work around this by transparently setting up a shadow framebuffer at the user's requested depth. The problem there is, if 565 is preferred but works, you're adding a format-conversion blit in the middle for no reason. If I could ask the kernel for the entire list of supported formats, I could only set up the shadow if it was necessary. - ajax ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/cirrus: rewrite and modernize driver.
Hi, > > So I can just remove cirrus_fb_dirty() and hook up > > drm_atomic_helper_dirtyfb() instead. Easy ;) > > You can even use drm_gem_fb_create_with_dirty() instead of > drm_gem_fb_create_with_funcs(). Ah, cool. /me happily continues removing code lines. thanks, Gerd PS: incremental fixups are at https://git.kraxel.org/cgit/linux/log/?h=drm-rewrite-cirrus ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/cirrus: rewrite and modernize driver.
Den 03.04.2019 10.53, skrev Gerd Hoffmann:
>>> +struct cirrus_device {
>>> + struct drm_device *dev;
>>
>> Why not embed drm_device? It's the latest rage :-)
>
> Sure, can do that.
>
>>> +void cirrus_pipe_update(struct drm_simple_display_pipe *pipe,
>>> + struct drm_plane_state *old_state)
>>> +{
>>> + struct drm_plane_state *state = pipe->plane.state;
>>> + struct drm_crtc *crtc = >crtc;
>>> + struct drm_rect rect;
>>> +
>>> + if (drm_atomic_helper_damage_merged(old_state, state, ))
>>> + cirrus_fb_blit_rect(pipe->plane.state->fb, );
>>> +
>>> + if (crtc->state->event) {
>>> + spin_lock_irq(>dev->event_lock);
>>> + drm_crtc_send_vblank_event(crtc, crtc->state->event);
>>> + spin_unlock_irq(>dev->event_lock);
>>> + crtc->state->event = NULL;
>>> + }
>>> +}
>
>>> +static int cirrus_fb_dirty(struct drm_framebuffer *fb,
>>> + struct drm_file *file_priv,
>>> + unsigned int flags, unsigned int color,
>>> + struct drm_clip_rect *clips,
>>> + unsigned int num_clips)
>>> +{
>>> + struct cirrus_device *cirrus = fb->dev->dev_private;
>>> +
>>> + if (cirrus->pipe.plane.state->fb != fb)
>>> + return 0;
>>> +
>>> + if (num_clips)
>>> + cirrus_fb_blit_clips(fb, clips, num_clips);
>>> + else
>>> + cirrus_fb_blit_fullscreen(fb);
>>> + return 0;
>>> +}
>>
>> Why not use the dirty helpers and implement dirty rect support in your
>> main plane update function?
>
> Dirty rect support is already there, see above.
>
> So I can just remove cirrus_fb_dirty() and hook up
> drm_atomic_helper_dirtyfb() instead. Easy ;)
>
You can even use drm_gem_fb_create_with_dirty() instead of
drm_gem_fb_create_with_funcs().
Noralf.
> cheers,
> Gerd
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/cirrus: rewrite and modernize driver.
> > +struct cirrus_device {
> > + struct drm_device *dev;
>
> Why not embed drm_device? It's the latest rage :-)
Sure, can do that.
> > +void cirrus_pipe_update(struct drm_simple_display_pipe *pipe,
> > + struct drm_plane_state *old_state)
> > +{
> > + struct drm_plane_state *state = pipe->plane.state;
> > + struct drm_crtc *crtc = >crtc;
> > + struct drm_rect rect;
> > +
> > + if (drm_atomic_helper_damage_merged(old_state, state, ))
> > + cirrus_fb_blit_rect(pipe->plane.state->fb, );
> > +
> > + if (crtc->state->event) {
> > + spin_lock_irq(>dev->event_lock);
> > + drm_crtc_send_vblank_event(crtc, crtc->state->event);
> > + spin_unlock_irq(>dev->event_lock);
> > + crtc->state->event = NULL;
> > + }
> > +}
> > +static int cirrus_fb_dirty(struct drm_framebuffer *fb,
> > + struct drm_file *file_priv,
> > + unsigned int flags, unsigned int color,
> > + struct drm_clip_rect *clips,
> > + unsigned int num_clips)
> > +{
> > + struct cirrus_device *cirrus = fb->dev->dev_private;
> > +
> > + if (cirrus->pipe.plane.state->fb != fb)
> > + return 0;
> > +
> > + if (num_clips)
> > + cirrus_fb_blit_clips(fb, clips, num_clips);
> > + else
> > + cirrus_fb_blit_fullscreen(fb);
> > + return 0;
> > +}
>
> Why not use the dirty helpers and implement dirty rect support in your
> main plane update function?
Dirty rect support is already there, see above.
So I can just remove cirrus_fb_dirty() and hook up
drm_atomic_helper_dirtyfb() instead. Easy ;)
cheers,
Gerd
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/cirrus: rewrite and modernize driver.
On Wed, Apr 3, 2019 at 9:23 AM Gerd Hoffmann wrote:
>
> Time to kill some bad sample code people are copying from ;)
>
> This is a complete rewrite of the cirrus driver. The cirrus_mode_set()
> function is pretty much the only function which is carried over largely
> unmodified. Everything else is upside down.
>
> It is a single monster patch. But given that it does some pretty
> fundamental changes to the drivers workflow and also reduces the code
> size by roughly 70% I think it'll still be alot easier to review than a
> longish baby-step patch series.
>
> Changes summary:
> - Given the small amout of video memory (4 MB) the cirrus device has
>the rewritten driver doesn't try to manage buffers there. Instead
>it will blit (memcpy) the active framebuffer to video memory.
> - All gem objects are stored in main memory and are manged using the
>new shmem helpers. ttm is out.
> - Only DRM_FORMAT_RGB565 (depth 16) is supported. The old driver does
>that too by default. There was a module parameter which enables 24/32
>bpp support and disables higher resolutions (due to cirrus hardware
>constrains). That parameter wasn't reimplemented.
> - The simple display pipeline is used.
> - The generic fbdev emulation is used.
> - It's a atomic driver now.
Sounds all awesome. Some tiny comments below, with those addressed
looks all good and gets my
Acked-by: Daniel Vetter
-Daniel
> Signed-off-by: Gerd Hoffmann
> ---
> drivers/gpu/drm/cirrus/cirrus_drv.h | 251 ---
> drivers/gpu/drm/cirrus/cirrus.c | 602 +
> drivers/gpu/drm/cirrus/cirrus_drv.c | 161 ---
> drivers/gpu/drm/cirrus/cirrus_fbdev.c | 309 -
> drivers/gpu/drm/cirrus/cirrus_main.c | 328 --
> drivers/gpu/drm/cirrus/cirrus_mode.c | 617 --
> drivers/gpu/drm/cirrus/cirrus_ttm.c | 343 --
> drivers/gpu/drm/cirrus/Kconfig| 2 +-
> drivers/gpu/drm/cirrus/Makefile | 3 -
> 9 files changed, 603 insertions(+), 2013 deletions(-)
> delete mode 100644 drivers/gpu/drm/cirrus/cirrus_drv.h
> create mode 100644 drivers/gpu/drm/cirrus/cirrus.c
> delete mode 100644 drivers/gpu/drm/cirrus/cirrus_drv.c
> delete mode 100644 drivers/gpu/drm/cirrus/cirrus_fbdev.c
> delete mode 100644 drivers/gpu/drm/cirrus/cirrus_main.c
> delete mode 100644 drivers/gpu/drm/cirrus/cirrus_mode.c
> delete mode 100644 drivers/gpu/drm/cirrus/cirrus_ttm.c
>
> diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.h
> b/drivers/gpu/drm/cirrus/cirrus_drv.h
> deleted file mode 100644
> index 828b150cdb20..
> --- a/drivers/gpu/drm/cirrus/cirrus_drv.h
> +++ /dev/null
> @@ -1,251 +0,0 @@
> -/*
> - * Copyright 2012 Red Hat
> - *
> - * This file is subject to the terms and conditions of the GNU General
> - * Public License version 2. See the file COPYING in the main
> - * directory of this archive for more details.
> - *
> - * Authors: Matthew Garrett
> - * Dave Airlie
> - */
> -#ifndef __CIRRUS_DRV_H__
> -#define __CIRRUS_DRV_H__
> -
> -#include
> -
> -#include
> -#include
> -
> -#include
> -#include
> -#include
> -#include
> -#include
> -
> -#include
> -
> -#define DRIVER_AUTHOR "Matthew Garrett"
> -
> -#define DRIVER_NAME"cirrus"
> -#define DRIVER_DESC"qemu Cirrus emulation"
> -#define DRIVER_DATE"20110418"
> -
> -#define DRIVER_MAJOR 1
> -#define DRIVER_MINOR 0
> -#define DRIVER_PATCHLEVEL 0
> -
> -#define CIRRUSFB_CONN_LIMIT 1
> -
> -#define RREG8(reg) ioread8(((void __iomem *)cdev->rmmio) + (reg))
> -#define WREG8(reg, v) iowrite8(v, ((void __iomem *)cdev->rmmio) + (reg))
> -#define RREG32(reg) ioread32(((void __iomem *)cdev->rmmio) + (reg))
> -#define WREG32(reg, v) iowrite32(v, ((void __iomem *)cdev->rmmio) + (reg))
> -
> -#define SEQ_INDEX 4
> -#define SEQ_DATA 5
> -
> -#define WREG_SEQ(reg, v) \
> - do {\
> - WREG8(SEQ_INDEX, reg); \
> - WREG8(SEQ_DATA, v); \
> - } while (0) \
> -
> -#define CRT_INDEX 0x14
> -#define CRT_DATA 0x15
> -
> -#define WREG_CRT(reg, v) \
> - do {\
> - WREG8(CRT_INDEX, reg); \
> - WREG8(CRT_DATA, v); \
> - } while (0) \
> -
> -#define GFX_INDEX 0xe
> -#define GFX_DATA 0xf
> -
> -#define WREG_GFX(reg, v) \
> - do {\
> - WREG8(GFX_INDEX, reg); \
> - WREG8(GFX_DATA, v); \
> - } while (0)
[PATCH] drm/cirrus: rewrite and modernize driver.
Time to kill some bad sample code people are copying from ;)
This is a complete rewrite of the cirrus driver. The cirrus_mode_set()
function is pretty much the only function which is carried over largely
unmodified. Everything else is upside down.
It is a single monster patch. But given that it does some pretty
fundamental changes to the drivers workflow and also reduces the code
size by roughly 70% I think it'll still be alot easier to review than a
longish baby-step patch series.
Changes summary:
- Given the small amout of video memory (4 MB) the cirrus device has
the rewritten driver doesn't try to manage buffers there. Instead
it will blit (memcpy) the active framebuffer to video memory.
- All gem objects are stored in main memory and are manged using the
new shmem helpers. ttm is out.
- Only DRM_FORMAT_RGB565 (depth 16) is supported. The old driver does
that too by default. There was a module parameter which enables 24/32
bpp support and disables higher resolutions (due to cirrus hardware
constrains). That parameter wasn't reimplemented.
- The simple display pipeline is used.
- The generic fbdev emulation is used.
- It's a atomic driver now.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/cirrus/cirrus_drv.h | 251 ---
drivers/gpu/drm/cirrus/cirrus.c | 602 +
drivers/gpu/drm/cirrus/cirrus_drv.c | 161 ---
drivers/gpu/drm/cirrus/cirrus_fbdev.c | 309 -
drivers/gpu/drm/cirrus/cirrus_main.c | 328 --
drivers/gpu/drm/cirrus/cirrus_mode.c | 617 --
drivers/gpu/drm/cirrus/cirrus_ttm.c | 343 --
drivers/gpu/drm/cirrus/Kconfig| 2 +-
drivers/gpu/drm/cirrus/Makefile | 3 -
9 files changed, 603 insertions(+), 2013 deletions(-)
delete mode 100644 drivers/gpu/drm/cirrus/cirrus_drv.h
create mode 100644 drivers/gpu/drm/cirrus/cirrus.c
delete mode 100644 drivers/gpu/drm/cirrus/cirrus_drv.c
delete mode 100644 drivers/gpu/drm/cirrus/cirrus_fbdev.c
delete mode 100644 drivers/gpu/drm/cirrus/cirrus_main.c
delete mode 100644 drivers/gpu/drm/cirrus/cirrus_mode.c
delete mode 100644 drivers/gpu/drm/cirrus/cirrus_ttm.c
diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.h
b/drivers/gpu/drm/cirrus/cirrus_drv.h
deleted file mode 100644
index 828b150cdb20..
--- a/drivers/gpu/drm/cirrus/cirrus_drv.h
+++ /dev/null
@@ -1,251 +0,0 @@
-/*
- * Copyright 2012 Red Hat
- *
- * This file is subject to the terms and conditions of the GNU General
- * Public License version 2. See the file COPYING in the main
- * directory of this archive for more details.
- *
- * Authors: Matthew Garrett
- * Dave Airlie
- */
-#ifndef __CIRRUS_DRV_H__
-#define __CIRRUS_DRV_H__
-
-#include
-
-#include
-#include
-
-#include
-#include
-#include
-#include
-#include
-
-#include
-
-#define DRIVER_AUTHOR "Matthew Garrett"
-
-#define DRIVER_NAME"cirrus"
-#define DRIVER_DESC"qemu Cirrus emulation"
-#define DRIVER_DATE"20110418"
-
-#define DRIVER_MAJOR 1
-#define DRIVER_MINOR 0
-#define DRIVER_PATCHLEVEL 0
-
-#define CIRRUSFB_CONN_LIMIT 1
-
-#define RREG8(reg) ioread8(((void __iomem *)cdev->rmmio) + (reg))
-#define WREG8(reg, v) iowrite8(v, ((void __iomem *)cdev->rmmio) + (reg))
-#define RREG32(reg) ioread32(((void __iomem *)cdev->rmmio) + (reg))
-#define WREG32(reg, v) iowrite32(v, ((void __iomem *)cdev->rmmio) + (reg))
-
-#define SEQ_INDEX 4
-#define SEQ_DATA 5
-
-#define WREG_SEQ(reg, v) \
- do {\
- WREG8(SEQ_INDEX, reg); \
- WREG8(SEQ_DATA, v); \
- } while (0) \
-
-#define CRT_INDEX 0x14
-#define CRT_DATA 0x15
-
-#define WREG_CRT(reg, v) \
- do {\
- WREG8(CRT_INDEX, reg); \
- WREG8(CRT_DATA, v); \
- } while (0) \
-
-#define GFX_INDEX 0xe
-#define GFX_DATA 0xf
-
-#define WREG_GFX(reg, v) \
- do {\
- WREG8(GFX_INDEX, reg); \
- WREG8(GFX_DATA, v); \
- } while (0) \
-
-/*
- * Cirrus has a "hidden" DAC register that can be accessed by writing to
- * the pixel mask register to reset the state, then reading from the register
- * four times. The next write will then pass to the DAC
- */
-#define VGA_DAC_MASK 0x6
-
-#define WREG_HDR(v)\
- do {
