Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48885 )
Change subject: nb/intel/hsw,soc/intel/{bdw,skl,apl},mb/*: unify dt panel settings ......................................................................
Patch Set 18: Code-Review+1
(4 comments)
https://review.coreboot.org/c/coreboot/+/48885/18/src/drivers/intel/gma/gma.... File src/drivers/intel/gma/gma.h:
https://review.coreboot.org/c/coreboot/+/48885/18/src/drivers/intel/gma/gma.... PS18, Line 14: graphics_soc_panel_init This is no longer true.
https://review.coreboot.org/c/coreboot/+/48885/18/src/drivers/intel/gma/gma.... PS18, Line 15: graphics_panel_settings To avoid naming collisions and for consistency with the struct above, how about renaming this to `i915_gpu_panel_info` or something like that?
NOTE: Just adding an `i915_` prefix is enough to soothe my OCD.
https://review.coreboot.org/c/coreboot/+/48885/18/src/mainboard/google/auron... File src/mainboard/google/auron/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/48885/18/src/mainboard/google/auron... PS18, Line 16: register "gpu_pch_backlight_pwm_hz" = "200" Personally, I would have moved this to the overridetrees in a separate commit, since it's a small, easy to review adjustment. It also makes this change more orthogonal (the modifications are the same for all devicetrees/overridetrees), which I appreciate when reviewing changes.
Given that this commit is buried in a patch train, I don't think updating this change is worth the effort, so I've marked this comment as resolved. I just wanted to tell you about it, so that you can keep this in mind for future changes.
https://review.coreboot.org/c/coreboot/+/48885/18/src/mainboard/lenovo/t440p... File src/mainboard/lenovo/t440p/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/48885/18/src/mainboard/lenovo/t440p... PS18, Line 9: .down_delay_ms = 500, : .cycle_delay_ms = 50, These two values are backwards