[Intel-gfx] [PATCH 04/19] drm/i915: move power domain macros to intel_pm.c

[Paulo Zanoni]

2014-02-20 16:21 GMT-03:00 Jesse Barnes <jbarnes at virtuousgeek.org>:
> On Tue, 18 Feb 2014 00:02:05 +0200
> Imre Deak <imre.deak at intel.com> wrote:
>
>> These macros are used only locally, so move them to the .c file.
>>
>> Also since logically the init power domain should be part of all power
>> wells add it to the always-on power wells too for consistency. Since
>> always-on power wells have noop handlers, this doesn't change the
>> functionality.

This patch should be split in two: one moves to .c, the other changes
the init power domain.

But I'm a little confused. I always understood that the "power_on"
domains were things that were always enabled on the hardware, so you
couldn't really turn them "off", no matter how many domains you put.
But the init power domain is different: when you get the init domain,
a lot of stuff can get enabled, and when you put it, a lot of stuff
can be disabled. By this logic, your patch would not make sense. So I
guess we probably have two different definitions for what is a
always_on power domain. I think we probably need to add some comments
in the source code to clarify the definitions of things, to avoid
further confusion :)

>>
>> Signed-off-by: Imre Deak <imre.deak at intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h | 10 ----------
>>  drivers/gpu/drm/i915/intel_pm.c | 21 +++++++++++++++++++--
>>  2 files changed, 19 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 796f971..de0c0e0 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -121,8 +121,6 @@ enum intel_display_power_domain {
>>       POWER_DOMAIN_NUM,
>>  };
>>
>> -#define POWER_DOMAIN_MASK (BIT(POWER_DOMAIN_NUM) - 1)
>> -
>>  #define POWER_DOMAIN_PIPE(pipe) ((pipe) + POWER_DOMAIN_PIPE_A)
>>  #define POWER_DOMAIN_PIPE_PANEL_FITTER(pipe) \
>>               ((pipe) + POWER_DOMAIN_PIPE_A_PANEL_FITTER)
>> @@ -130,14 +128,6 @@ enum intel_display_power_domain {
>>       ((tran) == TRANSCODER_EDP ? POWER_DOMAIN_TRANSCODER_EDP : \
>>        (tran) + POWER_DOMAIN_TRANSCODER_A)
>>
>> -#define HSW_ALWAYS_ON_POWER_DOMAINS (                \
>> -     BIT(POWER_DOMAIN_PIPE_A) |              \
>> -     BIT(POWER_DOMAIN_TRANSCODER_EDP))
>> -#define BDW_ALWAYS_ON_POWER_DOMAINS (                \
>> -     BIT(POWER_DOMAIN_PIPE_A) |              \
>> -     BIT(POWER_DOMAIN_TRANSCODER_EDP) |      \
>> -     BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER))
>> -
>>  enum hpd_pin {
>>       HPD_NONE = 0,
>>       HPD_PORT_A = HPD_NONE, /* PORT_A is internal */
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index db48d55..9a608f1 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -5384,6 +5384,23 @@ void i915_release_power_well(void)
>>  }
>>  EXPORT_SYMBOL_GPL(i915_release_power_well);
>>
>> +#define POWER_DOMAIN_MASK (BIT(POWER_DOMAIN_NUM) - 1)
>> +
>> +#define HSW_ALWAYS_ON_POWER_DOMAINS (                        \
>> +     BIT(POWER_DOMAIN_PIPE_A) |                      \
>> +     BIT(POWER_DOMAIN_TRANSCODER_EDP) |              \
>> +     BIT(POWER_DOMAIN_INIT))
>> +#define HSW_DISPLAY_POWER_DOMAINS (                          \
>> +     (POWER_DOMAIN_MASK & ~HSW_ALWAYS_ON_POWER_DOMAINS) |    \
>> +     BIT(POWER_DOMAIN_INIT))
>> +
>> +#define BDW_ALWAYS_ON_POWER_DOMAINS (                        \
>> +     HSW_ALWAYS_ON_POWER_DOMAINS |                   \
>> +     BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER))
>> +#define BDW_DISPLAY_POWER_DOMAINS (                          \
>> +     (POWER_DOMAIN_MASK & ~BDW_ALWAYS_ON_POWER_DOMAINS) |    \
>> +     BIT(POWER_DOMAIN_INIT))
>> +
>>  static struct i915_power_well i9xx_always_on_power_well[] = {
>>       {
>>               .name = "always-on",
>> @@ -5400,7 +5417,7 @@ static struct i915_power_well hsw_power_wells[] = {
>>       },
>>       {
>>               .name = "display",
>> -             .domains = POWER_DOMAIN_MASK & ~HSW_ALWAYS_ON_POWER_DOMAINS,
>> +             .domains = HSW_DISPLAY_POWER_DOMAINS,
>>               .is_enabled = hsw_power_well_enabled,
>>               .set = hsw_set_power_well,
>>       },
>> @@ -5414,7 +5431,7 @@ static struct i915_power_well bdw_power_wells[] = {
>>       },
>>       {
>>               .name = "display",
>> -             .domains = POWER_DOMAIN_MASK & ~BDW_ALWAYS_ON_POWER_DOMAINS,
>> +             .domains = BDW_DISPLAY_POWER_DOMAINS,
>>               .is_enabled = hsw_power_well_enabled,
>>               .set = hsw_set_power_well,
>>       },
>
> Reviewed-by: Jesse Barnes <jbarnes at virtuousgeek.org>
>
> --
> Jesse Barnes, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni