Re: [PATCH] DAC palette width retrieval

[Vladimir 'phcoder' Serbinenko]

> Now I'm not completely sure where does this belong.  I think it's somewhat
> generic and we can reasonably expect other OSes to want this information
> too, but I'm not completely sure.
For me putting these values to *_mask is ok but it may put some limits
for future cards but as payloads have no way to tell them more info it
should be a problem. Do any payloads need *_mask_offset? Even if so it
shouldn't stop the patch from going in if it fixes problems for linux
since only other payload where grub currently gives video info is xnu
and Darwin doesn't boot in anything with depth different from 32
anyway (or I couldn't figure out how to do it).
*_mask normally shouldn't be used in index color by internal grub
functions anyway. And if payload needs a special convention for
passing color info for indexed modes it should be handled by loader
Check however that this patch doesn't break gfxterm
>
> An alternative would be to do this query in the Linux loader, but this has
> the disadvantage that we're putting backend-specific code in the loader which
> aims to be completely backend-independant someday.
>
It is backend-independent with my framebuffer patch and I don't want
to go backwards.
> Also, tell me what you think about the "getset" hack.  It looks like a clean
> way of providing both functions in less space, but then I'm not sure if we
> really want both functions.  For now we only use the "set" one.  Then again,
> once you have "set" getting "get" comes really cheap.
>
It's ok but once we get my mm functions in and move this part to
vbe.mod splitting get and set will be a good idea

+         if (status != 0x004F)
It should be GRUB_VBE_STATUS_OK
+           /* 6 is default after mode reset.  */
+           width = 6;
Looks like a hack and some boards may have different value. Is there a
better way?
+         mode_info->red_mask_size = mode_info->green_mask_size
+           = mode_info->blue_mask_size = mode_info->rsvd_mask_size
+           = width;
Perhaps mode_info->rsvd_mask_size could be left at 0?
I don't think index colors have any alpha
-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git