[Intel-gfx] [PATCH 09/10] drm/i915/step: Add intel_step_name() helper

[Lucas De Marchi]

On Thu, Jul 08, 2021 at 09:16:16PM -0700, Matt Roper wrote:
>On Thu, Jul 08, 2021 at 04:18:20PM -0700, Anusha Srivatsa wrote:
>> Add a helper to convert the step info to string.
>> This is specifically useful when we want to load a specific
>> firmware for a given stepping/substepping combination.
>
>What if we use macros to generate the per-stepping code here as well as
>the stepping values in the enum?
>
>In intel_step.h:
>
>        #define STEPPING_NAME_LIST(func) \
>                func(A0)
>                func(A1)
>                func(A2)
>                func(B0)
>                ...
>
>        #define STEPPING_ENUM_VAL(name)  STEP_##name,
>
>        enum intel_step {
>                STEP_NONE = 0,
>                STEPPING_NAME_LIST(STEPPING_ENUM_VAL)
>                STEP_FUTURE,
>                STEP_FOREVER,
>        };
>
>and in intel_step.c:
>
>        #define STEPPING_NAME_CASE(name)        \
>                case STEP_##name:               \
>                        return #name;           \
>                        break;
>
>        const char *intel_step_name(enum intel_step step) {
>                switch(step) {
>                STEPPING_NAME_LIST(STEPPING_NAME_CASE)
>
>                default:
>                        return "**";
>                }
>        }
>
>This has the advantage that anytime a new stepping is added (in
>STEPPING_NAME_LIST) it will generate a new "STEP_XX" enum value and a
>new case statement to return "XX" as the name; we won't have to remember
>to update two separate places in the code.

my other idea in the first iterations of this patch was to turn the
stepping into u16 and then do something like (untested crap code below):

	#define make_step(a, b)	((a - 'A') << 8, (b - '0'))

	#define intel_step_name(s) ({
		char ret[3];
		ret[0] = ((s) >> 8) + 'A';
		ret[1] = ((s) & 0xff) + '0';
		ret[2] = '\0';
		ret;
	})

	enum intel_step {
		STEP_NONE = -1,
		STEP_A0 = make_step('A', '0'),
		...
	}

Or even not bother with the 'A'/'0' addition/subraction since 8 bits is
enough for all the letters and numbers.

If we keep it u8, then we are limited to step P7 (assuming we have 2
reserved entries at the end),. It may or may not be sufficient (it
currently is)

better? worse?

Lucas De Marchi

>
>
>Matt
>
>>
>> Suggested-by: Jani Nikula <jani.nikula at linux.intel.com>
>> Signed-off-by: Anusha Srivatsa <anusha.srivatsa at intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_step.c | 58 +++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_step.h |  1 +
>>  2 files changed, 59 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_step.c b/drivers/gpu/drm/i915/intel_step.c
>> index 99c0d3df001b..9af7f30b777e 100644
>> --- a/drivers/gpu/drm/i915/intel_step.c
>> +++ b/drivers/gpu/drm/i915/intel_step.c
>> @@ -182,3 +182,61 @@ void intel_step_init(struct drm_i915_private *i915)
>>
>>  	RUNTIME_INFO(i915)->step = step;
>>  }
>> +
>> +const char *intel_step_name(enum intel_step step) {
>> +	switch (step) {
>> +	case STEP_A0:
>> +		return "A0";
>> +		break;
>> +	case STEP_A1:
>> +		return "A1";
>> +		break;
>> +	case STEP_A2:
>> +		return "A2";
>> +		break;
>> +	case STEP_B0:
>> +		return "B0";
>> +		break;
>> +	case STEP_B1:
>> +		return "B1";
>> +		break;
>> +	case STEP_B2:
>> +		return "B2";
>> +		break;
>> +	case STEP_C0:
>> +		return "C0";
>> +		break;
>> +	case STEP_C1:
>> +		return "C1";
>> +		break;
>> +	case STEP_D0:
>> +		return "D0";
>> +		break;
>> +	case STEP_D1:
>> +		return "D1";
>> +		break;
>> +	case STEP_E0:
>> +		return "E0";
>> +		break;
>> +	case STEP_F0:
>> +		return "F0";
>> +		break;
>> +	case STEP_G0:
>> +		return "G0";
>> +		break;
>> +	case STEP_H0:
>> +		return "H0";
>> +		break;
>> +	case STEP_I0:
>> +		return "I0";
>> +		break;
>> +	case STEP_I1:
>> +		return "I1";
>> +		break;
>> +	case STEP_J0:
>> +		return "J0";
>> +		break;
>> +	default:
>> +		return "**";
>> +	}
>> +}
>> diff --git a/drivers/gpu/drm/i915/intel_step.h b/drivers/gpu/drm/i915/intel_step.h
>> index 3e8b2babd9da..2fbe51483472 100644
>> --- a/drivers/gpu/drm/i915/intel_step.h
>> +++ b/drivers/gpu/drm/i915/intel_step.h
>> @@ -43,5 +43,6 @@ enum intel_step {
>>  };
>>
>>  void intel_step_init(struct drm_i915_private *i915);
>> +const char *intel_step_name(enum intel_step step);
>>
>>  #endif /* __INTEL_STEP_H__ */
>> --
>> 2.32.0
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>-- 
>Matt Roper
>Graphics Software Engineer
>VTT-OSGC Platform Enablement
>Intel Corporation
>(916) 356-2795
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx at lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx