Re: [PATCH 4/4] Add support for getting EDID via EFI

["Vladimir 'φ-coder/phcoder' Serbinenko" <phcoder@gmail.com>]

On 08.02.2012 17:51, Matthew Garrett wrote:
> EFI gives a couple of defined methods for retrieving the EDID, so make use
> of them. Some Apple devices don't provide these but do stash the EDID in an
> nvram variable - grab it from there if it exists.
> ---
>   ChangeLog                 |    8 +++++
>   grub-core/video/efi_gop.c |   69 +++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 77 insertions(+), 0 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index 26d779b..d46b3d1 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,5 +1,13 @@
>   2012-02-08  Matthew Garrett<mjg@redhat.com>
>
> +  }
> +
> +  grub_memcpy (&edid_info, edid->edid, sizeof(edid_info));
> +
This code may copy more than actually present. You need to take minimum 
of both length and fill the rest with zeros
> +  return GRUB_ERR_NONE;
> +}
> +
> +static grub_err_t
> +grub_gop_get_preferred_mode (unsigned int *width, unsigned int *height)
> +{
> +  struct grub_video_edid_info edid_info;
> +
> +  if (grub_video_gop_get_edid(&edid_info) == GRUB_ERR_NONE)
> +    {
> +      if (grub_video_edid_checksum (&edid_info) == GRUB_ERR_NONE
> +	&&  grub_video_edid_preferred_mode (&edid_info, width, height)
> +	  == GRUB_ERR_NONE)
> +	return GRUB_ERR_NONE;
> +      else
> +	grub_dprintf("video", "invalid edid");
> +    }
> +  return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "cannot get preferred mode");
Please reuse the error which actually happened. E.g:
err = grub_video_gop_get_edid(&edid_info);
if (err)
    return err;
err =  grub_video_edid_checksum (&edid_info);
if (err)
   return err;
...
> +}
> +
> +static grub_err_t
>   grub_video_gop_setup (unsigned int width, unsigned int height,
>   		      unsigned int mode_type,
>   		      unsigned int mode_mask __attribute__ ((unused)))
> @@ -268,10 +319,18 @@ grub_video_gop_setup (unsigned int width, unsigned int height,
>     unsigned bpp;
>     int found = 0;
>     unsigned long long best_volume = 0;
> +  int preferred_mode = 0;
>
>     depth = (mode_type&  GRUB_VIDEO_MODE_TYPE_DEPTH_MASK)
>       >>  GRUB_VIDEO_MODE_TYPE_DEPTH_POS;
>
> +  if (width == 0&&  height == 0)
> +    {
> +      grub_gop_get_preferred_mode (&width,&height);
> +      if (grub_errno == GRUB_ERR_NONE)
> +	  preferred_mode = 1;
> +    }
> +
You need to discard error if you continue
>     /* Keep current mode if possible.  */
>     if (gop->mode->info)
>       {
> @@ -306,6 +365,15 @@ grub_video_gop_setup (


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko