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

[Atwood, Matthew S]

"dmc_payload is checked right before..." You are correct, there is no bug. It was my misunderstanding.

"What if someone doesn't really want to use DMC but still wants D3hot?"  or other great stuff that RPM can do without DMC firmware support?
________________________________________
From: Rodrigo Vivi [rodrigo.vivi at gmail.com]
Sent: Wednesday, June 14, 2017 1:02 PM
To: Deak, Imre
Cc: Atwood, Matthew S; 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: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