[coreboot-gerrit] [L] Change in coreboot[master]: drivers/apple: Add hybrid graphics driver

[Evgeny Zinoviev (Code Review)]

Attention is currently required from: Nico Huber, Martin L Roth, Angel Pons, Nicholas Chin, Patrick Rudolph.

Evgeny Zinoviev has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32719 )

Change subject: drivers/apple: Add hybrid graphics driver
......................................................................


Patch Set 28:

(10 comments)

This change is ready for review.

Commit Message:

https://review.coreboot.org/c/coreboot/+/32719/comment/c18e2e94_acd264e5 
PS14, Line 11: splitted
> split
> Done

Commit Message:

https://review.coreboot.org/c/coreboot/+/32719/comment/b608e273_bcbdfad5 
PS20, Line 15: ih
> hi
> Done

File src/drivers/apple/hybrid_graphics/acpi/gmux.asl:

https://review.coreboot.org/c/coreboot/+/32719/comment/4615384c_fd91fb63 
PS20, Line 12: 	    0x07FF, // Range Maximum
> Is the allocator aware of this fixed I/O resource?
> Please check out hybrid_graphics.c. I'm not sure I'm doing it right.

File src/drivers/apple/hybrid_graphics/chip.h:

https://review.coreboot.org/c/coreboot/+/32719/comment/9a117fe6_6b56de5e 
PS20, Line 14: gmux_indexed
> I'd make this a Kconfig option. […]
> Will do in next iterations.

File src/drivers/apple/hybrid_graphics/chip.h:

https://review.coreboot.org/c/coreboot/+/32719/comment/d6cb4a8b_e0756e04 
PS8, Line 24: #define HYBRID_GRAPHICS_DEFAULT_GPU HYBRID_GRAPHICS_INTEGRATED
> Make it a Kconfig?
> Will do in next iterations.

File src/drivers/apple/hybrid_graphics/gmux.h:

https://review.coreboot.org/c/coreboot/+/32719/comment/49f45f88_9d294f9a 
PS8, Line 43: #define GMUX_IOSTART                0x700
> It would be wise to reserve this for the allocator. For this, […]
> Please check out hybrid_graphics.c. I'm not sure I'm doing it right.

File src/drivers/apple/hybrid_graphics/hybrid_graphics.c:

https://review.coreboot.org/c/coreboot/+/32719/comment/52f0218b_4a4c8725 
PS7, Line 35: 	config = dev->chip_info;
> ok
> Done

https://review.coreboot.org/c/coreboot/+/32719/comment/d1be7987_3a3d55ca 
PS7, Line 37: 		printk(BIOS_INFO, "Hybrid graphics: Not installed\n");
> agreed. […]
> Done

File src/drivers/apple/hybrid_graphics/hybrid_graphics.c:

https://review.coreboot.org/c/coreboot/+/32719/comment/d24c23e6_17b9c87b 
PS8, Line 32: 	/* Don't confuse anyone else and disable the fake device */
> This is a copy-pasta from lenovo/hybrid_graphics driver, siro should know :) […]
> Done

File src/drivers/apple/hybrid_graphics/hybrid_graphics.c:

https://review.coreboot.org/c/coreboot/+/32719/comment/5cee364e_f37a7862 
PS23, Line 42: get_option
> Needs update to use type safe option API
> Done