[radeonsi] dpm: mc_reg_table slots

[radeonsi] dpm: mc_reg_table slots

[Sylvain BERTRAND]

Hi,

In si_populate_smc_acpi_state function, the acpi (emergency) state is a patched
version of the initial state. Then 'ACIndex = 0' for the acpi state (i.e.
setting it to SISLANDS_MCREGISTERTABLE_INITIAL_SLOT) seems misleading, since
ACIndex is already set to 0 (SISLANDS_MCREGISTERTABLE_INITIAL_SLOT) in
si_populate_smc_initial_state. That should be removed to avoid confusion.


Additionnally, that would mean the acpi state is not using its slot in
the mc_reg_table, *BUT* it is used by the ulv state! Indeed ACIndex is set to 1
(SISLANDS_MCREGISTERTABLE_ACPI_SLOT) in si_populate_ulv_state function.

We can see in si_populate_mc_reg_table that initial state, acpi state, ulv
state and driver state have their respective mc_reg_table slot filled.

Then, the previous code seems to make the ulv mc_reg_table slot unused and
useless to fill.

Moreover, is not following the SISLANDS_MCREGISTERTABLE_*_SLOT mc_reg_table
allocation.

Bug?

regards,

-- 
Sylvain

Re: [radeonsi] dpm: mc_reg_table slots

[Alex Deucher]

On Thu, Jan 23, 2014 at 8:57 AM, Sylvain BERTRAND <sylware@legeek.net> wrote:
> Hi,
>
> In si_populate_smc_acpi_state function, the acpi (emergency) state is a patched
> version of the initial state. Then 'ACIndex = 0' for the acpi state (i.e.
> setting it to SISLANDS_MCREGISTERTABLE_INITIAL_SLOT) seems misleading, since
> ACIndex is already set to 0 (SISLANDS_MCREGISTERTABLE_INITIAL_SLOT) in
> si_populate_smc_initial_state. That should be removed to avoid confusion.
>
>
> Additionnally, that would mean the acpi state is not using its slot in
> the mc_reg_table, *BUT* it is used by the ulv state! Indeed ACIndex is set to 1
> (SISLANDS_MCREGISTERTABLE_ACPI_SLOT) in si_populate_ulv_state function.
>
> We can see in si_populate_mc_reg_table that initial state, acpi state, ulv
> state and driver state have their respective mc_reg_table slot filled.
>
> Then, the previous code seems to make the ulv mc_reg_table slot unused and
> useless to fill.
>
> Moreover, is not following the SISLANDS_MCREGISTERTABLE_*_SLOT mc_reg_table
> allocation.
>
> Bug?

It looks like we only use slots 0 and 1 and the driver state slots.  I
confirmed catalyst does the same thing.  I suspect there was an
intention to use three states at some point, but it didn't end up
being useful.

Alex

>
> regards,
>
> --
> Sylvain
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel