[Intel-gfx] [PATCH] drm/i915: decouple runtime PM enablement from DMC presence

[Rodrigo Vivi]

On Wed, Jun 14, 2017 at 10:55 AM, Imre Deak <imre.deak at intel.com> wrote:
> On Wed, Jun 14, 2017 at 08:40:55PM +0300, Atwood, Matthew S wrote:
>> intel_csr_load_program can fail (if not supported by SoC, or if file
>> is size 0)
>
> Those are really just sanity checks, they can't happen normally. We
> should actually convert them to be WARNs.
>
>> and theres no conditional that it succeeds before releasing
>> power_put on POWER_DOMAIN_INIT, enabling runtime PM.
>
> It can't fail during loading, HAS_CSR() is already checked in
> intel_csr_ucode_init() and dev_priv->csr.dmc_payload is checked right
> before calling intel_csr_load_program().
>
>> As long as the driver *thinks* it has a valid path to a DMC firmware
>> this will execute.
>>
>> "without DMC loaded we want to keep runtime PM disabled" - Why?
>
> There is no reason to support that configuration.

What if someone doesn't really want to use DMC but still wants D3hot?

>
>> ________________________________________
>> From: Deak, Imre
>> Sent: Wednesday, June 14, 2017 10:33 AM
>> To: Atwood, Matthew S
>> Cc: intel-gfx at lists.freedesktop.org; marcheu at google.com; Matt Atwood
>> Subject: Re: [Intel-gfx] [PATCH] drm/i915: decouple runtime PM enablement from DMC presence
>>
>> On Wed, Jun 14, 2017 at 10:12:21AM -0700, matthew.s.atwood at intel.com wrote:
>> > From: Matt Atwood <matthew.s.atwood at intel.corp-partner.google.com>
>> >
>> > Runtime PM is disabled when DMC firmware is not present. Runtime PM is still
>> > enabled even if DMC firmware fails to load.
>>
>> Hm, that would be a bug, but I can't see how it can happen; could you
>> explain? We get a runtime PM reference in intel_csr_ucode_init() and
>> only put it if we loaded the firmware successfully.
>>
>> > This patch enables runtime PM to be enabled if DMC firmware is not present.
>>
>> Without DMC loaded we want to keep runtime PM disabled.
>>
>> --Imre
>>
>> >
>> > Signed-off-by: Matt Atwood <matthew.s.atwood at intel.corp-partner.google.com>
>> > ---
>> >  drivers/gpu/drm/i915/intel_csr.c | 7 +++----
>> >  1 file changed, 3 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
>> > index 965988f..3e4e705 100644
>> > --- a/drivers/gpu/drm/i915/intel_csr.c
>> > +++ b/drivers/gpu/drm/i915/intel_csr.c
>> > @@ -411,8 +411,6 @@ static void csr_load_work_fn(struct work_struct *work)
>> >       if (dev_priv->csr.dmc_payload) {
>> >               intel_csr_load_program(dev_priv);
>> >
>> > -             intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
>> > -
>> >               DRM_INFO("Finished loading DMC firmware %s (v%u.%u)\n",
>> >                        dev_priv->csr.fw_path,
>> >                        CSR_VERSION_MAJOR(csr->version),
>> > @@ -420,10 +418,11 @@ static void csr_load_work_fn(struct work_struct *work)
>> >       } else {
>> >               dev_notice(dev_priv->drm.dev,
>> >                          "Failed to load DMC firmware"
>> > -                        " [" FIRMWARE_URL "],"
>> > -                        " disabling runtime power management.\n");
>> > +                        " [" FIRMWARE_URL "]");
>> >       }
>> >
>> > +     intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
>> > +
>> >       release_firmware(fw);
>> >  }
>> >
>> > --
>> > 2.7.4
>> >
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx at lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br