On Fri, 28 Aug 2009 15:46:53 +1000
Dave Airlie <airl...@gmail.com> wrote:

> From: Dave Airlie <airl...@redhat.com>
> 
> Initially I always meant this code to be shared, but things
> ran away from me before I got to it.
> 
> This refactors the i915 and radeon kms fbdev interaction layers
> out into generic helpers + driver specific pieces.
> 
> It moves all the panic/sysrq enhancements to the core file,
> and stores a linked list of kernel fbs. This could possibly be
> improved to only store the fb which has fbcon on it for panics
> etc.
> 
> radeon retains some specific codes used for a big endian
> workaround.
> +static LIST_HEAD(kernel_fb_helper_list);
> +
> +bool drm_fb_helper_force_kernel_mode(void)
> +{
> +     int i = 0;
> +     bool ret, error = false;
> +     struct drm_fb_helper *helper;
> +
> +     if (list_empty(&kernel_fb_helper_list))
> +             return false;
> +
> +     list_for_each_entry(helper, &kernel_fb_helper_list,
> kernel_fb_list) {
> +             for (i = 0; i < helper->crtc_count; i++) {
> +                     struct drm_mode_set *mode_set =
> &helper->crtc_info[i].mode_set;
> +                     ret = drm_crtc_helper_set_config(mode_set);
> +                     if (ret)
> +                             error = true;
> +             }
> +     }
> +     return error;
> +}

Looks good; I came up with something similar while hacking on kdb this
week.

The only thing that's needed now is a lockless/atomic version of this
path.  I've got that for the mode_set_base part, but a full mode switch
is a little trickier since we go through more paths.

It's worth reminding people that the panic/sysrq stuff works best when
the server is run with -sharevts too, since that avoids bunch of the
Linux console junk that messes with input and printk.

> +static struct sysrq_key_op sysrq_drm_fb_helper_restore_op = {
> +     .handler = drm_fb_helper_sysrq,
> +     .help_msg = "force-fb(V)",
> +     .action_msg = "Restore framebuffer console",
> +};

In doing the KDB stuff I've been thinking it would be nice if this was
a toggle rather than a one way thing.  That way if your app is still
running you can go back to it.

> +
> +static void drm_fb_helper_on(struct fb_info *info)
> +{
> +     struct drm_fb_helper *fb_helper = info->par;
> +     struct drm_device *dev = fb_helper->dev;
> +     struct drm_crtc *crtc;
> +     struct drm_encoder *encoder;
> +     int i;
> +
> +     /*
> +      * For each CRTC in this fb, find all associated encoders
> +      * and turn them off, then turn off the CRTC.
> +      */

Cut & paste bugs on the comment (we're turning things on here).  This
one is actually mine I think, but don't feel obligated to preserve my
bugs. :)

Rest of it looks pretty good.  Any thoughts about how we might
eventually add some fuller fb functionality (like mode setting for
fbset)?  It seems like that could be mostly generic too...

Reviewed-by: Jesse Barnes <jbar...@virtuousgeek.org>
-- 
Jesse Barnes, Intel Open Source Technology Center

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with 
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

Reply via email to