<div dir="ltr">Hi again,<div><br></div><div>I'm talking about modifying radeon's side to mimic amdgpu's behavior, sorry for the confusion.</div><div><br></div><div>Cheers,</div><div>Alexandre</div><div><br><div class="gmail_quote"><div dir="ltr">On Fri, 12 Aug 2016 at 12:17 StDenis, Tom <<a href="mailto:Tom.StDenis@amd.com">Tom.StDenis@amd.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div dir="ltr"> <div style="font-size:12pt;color:#000000;background-color:#ffffff;font-family:Calibri,Arial,Helvetica,sans-serif"> <p>Hi Alex,</p> <p><br> </p> <p>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).</p> <p><br> </p> <p>Tom</p> <p><br> </p> <p><br> </p> <br> <br> <div style="color:rgb(0,0,0)"> <hr style="display:inline-block;width:98%"> <div dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> Alexandre Demers <<a href="mailto:alexandre.f.demers@gmail.com" target="_blank">alexandre.f.demers@gmail.com</a>><br> <b>Sent:</b> Friday, August 12, 2016 12:11<br> <b>To:</b> StDenis, Tom; Freedesktop - AMD-gfx<br> <b>Cc:</b> Alexander Deucher<br> <b>Subject:</b> Re: radeon VS amdgpu: _afmt_init() behavior if kzalloc fails</font> <div> </div> </div></div></div></div><div dir="ltr"><div style="font-size:12pt;color:#000000;background-color:#ffffff;font-family:Calibri,Arial,Helvetica,sans-serif"><div style="color:rgb(0,0,0)"> <div> <div dir="ltr">Thanks for the quick answer Tom. <span style="line-height:1.5">I was thinking mostly as you do, but before sending the patch to the list, I wanted to be sure it was worth it.</span> <div><br> <div>I'll send the patch later then, unless someone says otherwise.</div> <div><br> </div> <div>Cheers,</div> <div>Alexandre</div> </div> </div> <br> <div class="gmail_quote"> <div dir="ltr">On Fri, 12 Aug 2016 at 11:50 StDenis, Tom <<a href="mailto:Tom.StDenis@amd.com" target="_blank">Tom.StDenis@amd.com</a>> wrote:<br> </div> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div dir="ltr"> <div style="font-size:12pt;color:#000000;background-color:#ffffff;font-family:Calibri,Arial,Helvetica,sans-serif"> <p>Hi,</p> <p><br> </p> <p>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.</p> <p><br> </p> <p>Generally speaking if you fail to kmalloc a few bytes you've got bigger problems to worry about than your audio not working ideally.</p> <p><br> </p> <p>Tom</p> <br> <br> <div style="color:rgb(0,0,0)"> <hr style="display:inline-block;width:98%"> <div dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> amd-gfx <<a href="mailto:amd-gfx-bounces@lists.freedesktop.org" target="_blank">amd-gfx-bounces@lists.freedesktop.org</a>> on behalf of Alexandre Demers <<a href="mailto:alexandre.f.demers@gmail.com" target="_blank">alexandre.f.demers@gmail.com</a>><br> <b>Sent:</b> Friday, August 12, 2016 11:43<br> <b>To:</b> Freedesktop - AMD-gfx<br> <b>Cc:</b> Alexander Deucher<br> <b>Subject:</b> radeon VS amdgpu: _afmt_init() behavior if kzalloc fails</font> <div> </div> </div> </div> </div> </div> <div dir="ltr"> <div style="font-size:12pt;color:#000000;background-color:#ffffff;font-family:Calibri,Arial,Helvetica,sans-serif"> <div style="color:rgb(0,0,0)"> <div> <div dir="ltr">Hi, <div><br> </div> <div>While comparing radeon's radeon_afmt_init() to dce_vX_0_afmt_init() on the amdgpu's side, I saw that on the latter:</div> <div>1- when kzalloc fails to allocate mem for all afmt, an ENOMEM is returned</div> <div>2- all previously allocated afmt are freed</div> <div><br> </div> <div>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.</div> <div><br> </div> <div>Moreover, returning an ENOMEM prevents any other function calls in <span style="font-size:13.3333px;line-height:normal">dce_vX_0_</span><span style="font-size:13.3333px;line-height:normal">sw_init() on the amdgpu's side, while it continues on the radeon's side.</span><br> </div> <div><span style="font-size:13.3333px;line-height:normal"><br> </span></div> <div><span style="font-size:13.3333px;line-height:normal">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())?</span></div> <div><span style="font-size:13.3333px;line-height:normal"><br> </span></div> <div><span style="font-size:13.3333px;line-height:normal">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.</span></div> <div><span style="font-size:13.3333px;line-height:normal"><br> </span></div> <div><span style="font-size:13.3333px;line-height:normal">Cheers,</span></div> <div><span style="font-size:13.3333px;line-height:normal">Alexandre Demers</span></div> </div> </div> </div> </div> </div> </blockquote> </div> </div> </div></div></div></blockquote></div></div></div>