Thomas Zimmermann <[email protected]> writes:
> The device handle of the GOP device is required to retrieve the
> correct EDID data. Find the handle instead of the GOP data. Still
> return the GOP data in the function arguments, as we already looked
> it up.
>
> Signed-off-by: Thomas Zimmermann <[email protected]>
> ---
> drivers/firmware/efi/libstub/gop.c | 27 +++++++++++++++++----------
> 1 file changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/gop.c
> b/drivers/firmware/efi/libstub/gop.c
> index 3785fb4986b4..fd32be8dd146 100644
> --- a/drivers/firmware/efi/libstub/gop.c
> +++ b/drivers/firmware/efi/libstub/gop.c
> @@ -402,12 +402,13 @@ setup_pixel_info(struct screen_info *si, u32
> pixels_per_scan_line,
> }
> }
>
> -static efi_graphics_output_protocol_t *find_gop(unsigned long num,
> - const efi_handle_t handles[])
> +static efi_handle_t find_handle_with_primary_gop(unsigned long num, const
> efi_handle_t handles[],
> + efi_graphics_output_protocol_t
> **found_gop)
> {
> efi_graphics_output_protocol_t *first_gop;
> - efi_handle_t h;
> + efi_handle_t h, first_gop_handle;
>
> + first_gop_handle = NULL;
> first_gop = NULL;
>
I think the logic of this function could be simplified if you remove some
of the variables. For example, I don't think you need a fist_gop variable
anymore now that you are passing a found_gop variable as a function param.
> for_each_efi_handle(h, handles, num) {
> @@ -442,19 +443,25 @@ static efi_graphics_output_protocol_t
> *find_gop(unsigned long num,
> */
> status = efi_bs_call(handle_protocol, h,
> &EFI_CONSOLE_OUT_DEVICE_GUID, &dummy);
> - if (status == EFI_SUCCESS)
> - return gop;
> -
> - if (!first_gop)
> + if (status == EFI_SUCCESS) {
> + if (found_gop)
> + *found_gop = gop;
> + return h;
> + } else if (!first_gop_handle) {
> + first_gop_handle = h;
> first_gop = gop;
You can just assign *found_gop = gop here...
> + }
> }
>
> - return first_gop;
> + if (found_gop)
> + *found_gop = first_gop;
...and then this assignment won't be needed anynmore.
> + return first_gop_handle;
Also, given that you are calling first_gop_handle to the variable to
store the first gop handle, I would for consistency name the parameter
fist_gop and just drop the local variable with the same name.
But I agree with the general logic of the patch, so regardless of what
you prefer to do:
Reviewed-by: Javier Martinez Canillas <[email protected]>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
