Re: [git pull] drm for rc1

From: Linus Torvalds
Date: Wed Jan 12 2011 - 10:56:41 EST


On Wed, Jan 12, 2011 at 5:32 AM, James Simmons <jsimmons@xxxxxxxxxxxxx> wrote:
>
> Okay. The nouveau driver also uses the pitch as well. It
> really should be using the pitch field from drm_framebuffer instead of the
> line_length from fb_fix_screeninfo. This patch is just to make sure this
> is the issue. I will submit another patch later that uses
> drm_fb_framebuffer's pitch field. As for the visual unfortunely their is
> no real mapping between drm and fbdev.

Why do you want to remove the drm_fb_helper_fill_fix() call? Quite
frankly, you're then replacing it with open-coding the function
partially:

> +       info->fix.visual = fb->depth == 8 ? FB_VISUAL_PSEUDOCOLOR :
> +                FB_VISUAL_TRUECOLOR;
> +       info->fix.line_length = fb->pitch;
>        drm_fb_helper_fill_var(info, &nfbdev->helper, sizes->fb_width, sizes->fb_height);

Which seems to be just a regression. Why not just call
"drm_fb_helper_fill_fix()" here like we used to?

IOW, I'm inclined to just do the revert. The "fix" clearly breaks
things, and now you're adding random parts of the function back rather
than just calling the "fill_fix()" function like things used to. Why?

The commit message in dfe63bb0ad98 doesn't support any of these hacks
- it just seems to say that drm_fb_helper_fill_fix() should also have
been called from setcolreg().

So why don't we just revert the commit and instead add that
drm_fb_helper_fill_fix() to setcolreg()?

Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/