Change in coreboot[master]: soc/intel/common: Implement ACPI CPPCv3 package to support hybrid core

[Sridhar Siricilla (Code Review)]

Attention is currently required from: Arthur Heymans, Nico Huber, Paul Menzel, Tim Wawrzynczak, Ravindra, Subrata Banik, Michael Niewöhner, Patrick Rudolph.
Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59359 )

Change subject: soc/intel/common: Implement ACPI CPPCv3 package to support hybrid core
......................................................................

Patch Set 5:

(4 comments)

Patchset:

PS3:
> Looking at code and referring to the sample implementation, I think below are only changes that we n […]

Agree.
> . Don't need to create any new cpu_index() function that returns sorted cpu index.

Kindly note CPPC3 package entries are same across all cores except nominal performance which depends on the core type (small or big).
Here only differentiator is nominal performance which has to be assigned to 0xCE(MSR).offset:8. Do you mean coreboot should set the MSR 0xCE with calculated nominal performance (based on core type) in the MP code instead of generating ACPI code for this element?

File src/soc/intel/common/block/acpi/cpu_hybrid.c:

https://review.coreboot.org/c/coreboot/+/59359/comment/ebce0226_61becb4e 
PS3, Line 63: get_cpu_type_bitmask
> global_cpu_type_bitmask can be accessed by acpi_get_cpu_hybrid_info() directly, so get_cpu_type_bitm […]

Ack

https://review.coreboot.org/c/coreboot/+/59359/comment/5d803081_ab056c6f 
PS3, Line 152: acpigen_write_dword(core_id);
> why not call XPPC using the actual information you want: big or small core instead of passing the co […]

Correct, I have updated the patch that doesn't use bitmask.

https://review.coreboot.org/c/coreboot/+/59359/comment/b2d5187d_a0031993 
PS3, Line 98: void acpi_write_xppc_method(int core_id)
            : {
            : 	struct cpu_hybrid cpu_hybrid_info;
            : 
            : 	acpi_get_cpu_hybrid_info(&cpu_hybrid_info);
            : 	acpigen_write_method(XPPC_PACKAGE_NAME, 1);
            : 
            : 	if (cpu_is_nominal_freq_supported()) {
            : 
            : 		/*
            : 		 * If Nominal Frequency is supported, update Nominal Frequency and set core's
            : 		 * Nominal Performance to a scaling factor which depends on core type(big or
            : 		 * small).
            : 		 */
            : 		acpigen_set_package_element_int(CPPC_PACKAGE_NAME, 22,
            : 				cpu_hybrid_info.nominal_freq);
            : 
            : 		/* LOCAL2_OP = (CORE >> core_id) & 1 */
            : 		acpigen_emit_byte(SHIFT_RIGHT_OP);
            : 		acpigen_write_integer(cpu_hybrid_info.type_mask);
            : 		acpigen_emit_byte(ARG0_OP);
            : 		acpigen_emit_byte(LOCAL1_OP);
            : 		acpigen_write_and(LOCAL1_OP, ONE_OP, LOCAL2_OP);
            : 
            : 		/*
            : 		 * If core id is big Core, then set big core's scaling factor
            : 		 * to core's nominal frequency otherwise set small core's scaling factor.
            : 		 */
            : 		acpigen_write_if_lequal_op_int(LOCAL2_OP, 1);
            : 		acpigen_set_package_element_int(CPPC_PACKAGE_NAME, 3,
            : 				cpu_hybrid_info.big_core_nom_perf);
            : 		acpigen_write_else();
            : 		acpigen_set_package_element_int(CPPC_PACKAGE_NAME, 3,
            : 				cpu_hybrid_info.small_core_nom_perf);
            : 		acpigen_pop_len();
            : 	}
            : 
            : 	acpigen_emit_byte(RETURN_OP);
            : 	acpigen_emit_namestring(CPPC_PACKAGE_NAME);
            : 	acpigen_pop_len();
            : }
            : 
            : void acpigen_write_CPPC_hybrid_method(int core_id)
            : {
            : 	char scope[16];
            : 
            : 	if (core_id == 0)
            : 		snprintf(scope, 12, XPPC_PACKAGE_NAME, 0);
            : 	else
            : 		snprintf(scope, 16, CONFIG_ACPI_CPU_STRING "." XPPC_PACKAGE_NAME, 0);
            : 
            : 	acpigen_write_method("_CPC", 0);
            : 	acpigen_emit_byte(RETURN_OP);
            : 	acpigen_emit_namestring(scope);
            : 	acpigen_write_dword(core_id);
            : 	acpigen_pop_len();
> I'm a bit confused. […]
> Each CPU has _CPC method and they all use the implementation called XPPC_PACKAGE_NAME of CPU0? Might just as well generate it for each CPU and drop that weird bitmask code altogether.

Correct, I have updated the patch without using the cpu type bit mask.