[Intel-gfx] [PATCH] drm/i915/skl: replace csr_mutex by completion in csr firmware loading

[Dave Gordon]

On 15/06/15 11:07, Daniel Vetter wrote:
> On Thu, Jun 04, 2015 at 03:36:32PM +0100, Dave Gordon wrote:
>> On 04/06/15 06:59, Sagar Arun Kamble wrote:
>>>
>>> Hi Daniel,
>>>
>>> We already are grabbing RPM reference before start of DMC FW load and
>>> release post load completion.
>>>
>>> DC5/6 can happen without Runtime PM as well. So we need to wait for CSR
>>> FW load for some time once we disable PW2.
>>>
>>> Having completion instead of csr.lock+csr.state is  correct thing to do.
>>>
>>> Thanks,
>>> Sagar
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>> Hi guys,
>>
>> we have a new unified loader for GuC and HuC, which we intend should be
>> usable for any other microcontrollers that might be added in future. So
>> it would be good to consider using it for the DMC/CSR/etc.
>>
>> The most-recently posted version was Alex's on 2015-04-29, but we're
>> just about to re-post an updated patchset for command submission via the
>> GuC, which will include the both unified firmware loading module and the
>> GuC-specific part of the loader.
>>
>> Please have a look at these patches and let me know whether there's
>> anything that would help with using the unified code for your uC(s).
>>
>> http://lists.freedesktop.org/archives/intel-gfx/2015-April/065575.html
>> http://lists.freedesktop.org/archives/intel-gfx/2015-April/065583.html
>>
>> One thing that may not be obvious from the posted patches is that the
>> uC-specific code can override the way the f/w image is saved e.g. if the
>> loaded file image contains sections that are only needed once, they need
>> not be saved; more generally, if the way that the data in the image file
>> is organised doesn't match the way the data is used, the loader can
>> shuffle the content around for the convenience of the final consumer.
>> (We needed that at one stage, as the GuC firmware format didn't match
>> the constraints of the GuC DMA engine; but the format has been fixed
>> since, so the data-shuffler is not part of the public patch sequence).
> 
> Hm just looked at the generic firmware loader and not convinced this is a
> good idea at all. As this discussion here shows synchronization here is a
> tricky business. And in general driver load ordering is full of peril.
> Trying to force a common abstraction for something rather trivial (we have
> completions for the most basic case already) will only result in pains
> trying to work around the not-quite-fitting infrastructure.
> 
> In my experience trying to extract common code at all costs is harmful
> way too often.
> -Daniel

That version is now obsolete; the new version of the generic loader code
is in the GuC submission series that I just posted.

Thanks,
.Dave.