On Wed, 07 Jul 2021, Jani Nikula <jani.nik...@linux.intel.com> wrote:
> On Tue, 06 Jul 2021, Anusha Srivatsa <anusha.sriva...@intel.com> wrote:
>> Instead of adding new table for every new platform, lets ues
>> the stepping info from RUNTIME_INFO(dev_priv)->step
>> This patch uses RUNTIME_INFO->step only for recent
>> platforms.
>>
>> Patches that follow this will address this change for
>> remaining platforms + missing platforms.
>>
>> Signed-off-by: Anusha Srivatsa <anusha.sriva...@intel.com>
>> ---
>> drivers/gpu/drm/i915/display/intel_dmc.c | 61 +++++++++++++++++++++---
>> 1 file changed, 54 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
>> b/drivers/gpu/drm/i915/display/intel_dmc.c
>> index f8789d4543bf..a38720f25910 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dmc.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dmc.c
>> @@ -266,10 +266,12 @@ static const struct stepping_info icl_stepping_info[]
>> = {
>> };
>>
>> static const struct stepping_info no_stepping_info = { '*', '*' };
>> +struct stepping_info *display_step;
>
> We can't have driver specific mutable data for this. Almost everything
> has to be either device specific or const. The above would be shared
> between all devices.
I think the solution to your problem is two-fold.
First, I think you should add a *generic* function in intel_step.c to
get the chars or a string for an enum intel_step. Maybe a string,
because it'll also be useful for logging?
const char *intel_step_name(enum intel_step step)
{
switch (step) {
case STEP_A0:
return "A0";
case STEP_B0;
/* etc ... */
default:
return "??";
}
}
Second, I think you should modify intel_get_stepping_info() to let you
pass in the struct stepping_info pointer to fill in. Then you can have a
local struct stepping_info si variable in parse_dmc_fw(). We don't need
to store the data anywhere, it's only used once.
static void intel_get_stepping_info(struct drm_i915_private *dev_priv,
struct stepping_info *si)
There you'd do something like:
const char *step_name = intel_step_name(step);
si->stepping = step_name[0];
si->stepping = step_name[1];
And potentially handle the ?? case separately. Something along those
lines.
BR,
Jani.
>
> BR,
> Jani.
>
>>
>> static const struct stepping_info *
>> intel_get_stepping_info(struct drm_i915_private *dev_priv)
>> {
>> + struct intel_step_info step = RUNTIME_INFO(dev_priv)->step;
>> const struct stepping_info *si;
>> unsigned int size;
>>
>> @@ -282,15 +284,60 @@ intel_get_stepping_info(struct drm_i915_private
>> *dev_priv)
>> } else if (IS_BROXTON(dev_priv)) {
>> size = ARRAY_SIZE(bxt_stepping_info);
>> si = bxt_stepping_info;
>> - } else {
>> - size = 0;
>> - si = NULL;
>> }
>>
>> - if (INTEL_REVID(dev_priv) < size)
>> - return si + INTEL_REVID(dev_priv);
>> -
>> - return &no_stepping_info;
>> + if (IS_ICELAKE(dev_priv) || IS_SKYLAKE(dev_priv) ||
>> IS_BROXTON(dev_priv))
>> + return INTEL_REVID(dev_priv) < size ? si +
>> INTEL_REVID(dev_priv) : &no_stepping_info;
>> +
>> + else {
>> + switch (step.display_step) {
>> + case STEP_A0:
>> + display_step->stepping = 'A';
>> + display_step->substepping = '0';
>> + break;
>> + case STEP_A2:
>> + display_step->stepping = 'A';
>> + display_step->substepping = '2';
>> + break;
>> + case STEP_B0:
>> + display_step->stepping = 'B';
>> + display_step->substepping = '0';
>> + break;
>> + case STEP_B1:
>> + display_step->stepping = 'B';
>> + display_step->substepping = '1';
>> + break;
>> + case STEP_C0:
>> + display_step->stepping = 'C';
>> + display_step->substepping = '0';
>> + break;
>> + case STEP_D0:
>> + display_step->stepping = 'D';
>> + display_step->substepping = '0';
>> + break;
>> + case STEP_D1:
>> + display_step->stepping = 'D';
>> + display_step->substepping = '1';
>> + break;
>> + case STEP_E0:
>> + display_step->stepping = 'E';
>> + display_step->substepping = '0';
>> + break;
>> + case STEP_F0:
>> + display_step->stepping = 'F';
>> + display_step->substepping = '0';
>> + break;
>> + case STEP_G0:
>> + display_step->stepping = 'G';
>> + display_step->substepping = '0';
>> + break;
>> + default:
>> + display_step->stepping = '*';
>> + display_step->substepping = '*';
>> + break;
>> + }
>> + }
>> + return display_step;
>> }
>>
>> static void gen9_set_dc_state_debugmask(struct drm_i915_private *dev_priv)
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
