radeon VS amdgpu: _afmt_init() behavior if kzalloc fails

[Alexandre Demers]

Maybe, but it's already done, so. ;)

By the way, I haven't sent my patches yet for SI's unimplemented dce
functions, I'm still cleaning/tuning them up between other tasks at home, i
t should be sent in the next few days. Lesson learned though: I should have
planned what I wanted to do one thing at a time instead of many things at
the same time... It would have been more efficient for patch creation.

Cheers,
Alexandre

On Fri, 12 Aug 2016 at 13:41 StDenis, Tom <Tom.StDenis at amd.com> wrote:

> Hi Alexandre,
>
>
> That's probably fine.  Though if it's touching on ASICs that amdgpu is
> meant to support and it's not a critical bugfix (or API update) maybe time
> is better spent getting support on the amdgpu side going for that hardware
> instead.
>
>
> Cheers,
>
> Tom
>
>
> ------------------------------
> *From:* Alexandre Demers <alexandre.f.demers at gmail.com>
> *Sent:* Friday, August 12, 2016 13:39
>
> *To:* StDenis, Tom; Freedesktop - AMD-gfx
> *Cc:* Alexander Deucher
> *Subject:* Re: radeon VS amdgpu: _afmt_init() behavior if kzalloc fails
> Hi again,
>
> I'm talking about modifying radeon's side to mimic amdgpu's behavior,
> sorry for the confusion.
>
> Cheers,
> Alexandre
>
> On Fri, 12 Aug 2016 at 12:17 StDenis, Tom <Tom.StDenis at amd.com> wrote:
>
>> Hi Alex,
>>
>>
>> I'm unsure of what patch you have specifically.  Are you suggesting to
>> modify the radeon side to mimic the behaviour?  Or to revert the amdgpu
>> changes?  I  think reverting the amdgpu side won't go over well.  It's
>> churn and the current approach is arguably better for a number of reasons
>> namely it's better defined behaviour and generally speaking if during
>> module init a kmalloc of 100 bytes fails something bad is happening and you
>> want to abort init anyways (so failing to load just because part of DCE
>> fails is probably a good thing).
>>
>>
>> Tom
>>
>>
>>
>>
>>
>> ------------------------------
>> *From:* Alexandre Demers <alexandre.f.demers at gmail.com>
>> *Sent:* Friday, August 12, 2016 12:11
>> *To:* StDenis, Tom; Freedesktop - AMD-gfx
>> *Cc:* Alexander Deucher
>> *Subject:* Re: radeon VS amdgpu: _afmt_init() behavior if kzalloc fails
>>
>> Thanks for the quick answer Tom. I was thinking mostly as you do, but
>> before sending the patch to the list, I wanted to be sure it was worth it.
>>
>> I'll send the patch later then, unless someone says otherwise.
>>
>> Cheers,
>> Alexandre
>>
>> On Fri, 12 Aug 2016 at 11:50 StDenis, Tom <Tom.StDenis at amd.com> wrote:
>>
>>> Hi,
>>>
>>>
>>> I wrote those changes back in March as part of a cleanup sweep.  The
>>> idea being was that if some had failed the state of the driver would be
>>> unknown so it was cleaner to simply abort allocating any memory and set all
>>> of the pointers to NULL.
>>>
>>>
>>> Generally speaking if you fail to kmalloc a few bytes you've got bigger
>>> problems to worry about than your audio not working ideally.
>>>
>>>
>>> Tom
>>>
>>>
>>> ------------------------------
>>> *From:* amd-gfx <amd-gfx-bounces at lists.freedesktop.org> on behalf of
>>> Alexandre Demers <alexandre.f.demers at gmail.com>
>>> *Sent:* Friday, August 12, 2016 11:43
>>> *To:* Freedesktop - AMD-gfx
>>> *Cc:* Alexander Deucher
>>> *Subject:* radeon VS amdgpu: _afmt_init() behavior if kzalloc fails
>>>
>>> Hi,
>>>
>>> While comparing radeon's radeon_afmt_init() to dce_vX_0_afmt_init() on
>>> the amdgpu's side, I saw that on the latter:
>>> 1- when kzalloc fails to allocate mem for all afmt, an ENOMEM is returned
>>> 2- all previously allocated afmt are freed
>>>
>>> So on the amdgpu's side, it is an "all or none allocated" situation,
>>> while on the radeon's side, some afmt may be allocated and initialized.
>>>
>>> Moreover, returning an ENOMEM prevents any other function calls in
>>> dce_vX_0_sw_init() on the amdgpu's side, while it continues on the
>>> radeon's side.
>>>
>>> What is the expected behavior here? Should we rewind the memory
>>> allocation as it is done under amdgpu when we can't allocate memory for all
>>> afmt or is it OK to do as it is done on radeon? Should we stop any
>>> remaining steps in radeon_modeset_init() if it fails (thus, returning a
>>> ENOMEM from radeon_afmt_init())?
>>>
>>> The patch is already ready if needed, I could send it later from home if
>>> the amdgpu's behavior is the one that we are looking for.
>>>
>>> Cheers,
>>> Alexandre Demers
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20160812/b066bf31/attachment-0001.html>