[Mesa-dev] [Mesa-stable] [PATCH] radeon/vce: move feedback command inside of destroy function

[Mark Janes]

Emil Velikov <emil.l.velikov at gmail.com> writes:

> On 5 April 2018 at 20:33, Mark Janes <mark.a.janes at intel.com> wrote:
>> Emil Velikov <emil.l.velikov at gmail.com> writes:
>>
>>> On 4 April 2018 at 22:50, Mark Janes <mark.a.janes at intel.com> wrote:
>>>> Leo Liu <leo.liu at amd.com> writes:
>>>>
>>>>> On 04/04/2018 12:40 PM, Mark Janes wrote:
>>>>>> Leo Liu <leo.liu at amd.com> writes:
>>>>>>
>>>>>>> On the CI family, firmware requires the destory command have to be the
>>>>>>> last command in the IB, moving feedback command after destroy is causing
>>>>>>> issues on CI cards, so we have to keep the previous logic that moves
>>>>>>> destroy back to the last command.
>>>>>>>
>>>>>>> But as the original issue fixed previously, with the newer family like Vega10,
>>>>>>> feedback command have to be included inside of the task info command along
>>>>>>> with destroy command.
>>>>>>>
>>>>>>> Fixes: 6d74cb25("radeon/vce: move destroy command before feedback command")
>>>>>>>
>>>>>>> Signed-off-by: Leo Liu <leo.liu at amd.com>
>>>>>>> Cc: mesa-stable at lists.freedesktop.org
>>>>>> These tags seem ambiguous to me.  If this commit fixes a specific
>>>>>> commit, then the patch should be applied only to stable branches which
>>>>>> contain that commit.
>>>>>>
>>>>>> However, the mesa-stable CC caused this patch to be applied to 17.3,
>>>>>> which does *not* contain the broken patch.
>>>>>>
>>>>>> Leo: did you intend for the mesa-stable CC to cause this patch to be
>>>>>> applied to older stable branches?
>>>>> I would like to have this patch apply to branches "17.2", "17.3",
>>>>> "18.0", which got patch titled "radeon/vce: move destroy command before
>>>>> feedback command"
>>>>
>>>> Ok, I understand now.  You cc'd a buggy patch to stable, and the bug was
>>>> shipped in 17.3.1.
>>>>
>>> May I suggest phrasing things less personally. Mistakes happen, so
>>> let's work in providing suggestions for improvement as opposed to "you
>>> did X/Y".
>>
>> Thank you for the feedback.  I was trying to state the facts, but I
>> understand how this could be read as a criticism.
>>
> Does that mean you tested radeon/vce and observed the breakage?
> </cheeky>
>
>> As you say, mistakes happen -- and when they happen on the stable
>> branches, there is very little to protect the end users.  Could we
>> enhance automation to prevent this situation?  For example:
>>
> Consistent testing/reporting is needed. I believe I've mentioned if before:
>
> You are the only one who consistently provides feedback about the state.
> There have been individuals to report, while I'm very grateful these
> reports are very rare and far between.
>
> Approx 4 years ago Carl suggested another alternative. Roughly put:
>
> Driver specific patches are _omitted_ unless team member has
> explicitly tested them.
> Needless to say plan did not go forward - see the whole thread [1].
>
> One thing that it had in common with recent discussion is a
> tester/frontman/maintainer/etc for each team.
>
> Having such a person alongside the actual testing is optional, yet
> _highly_ recommended.
>
> As you know Intel's team is the largest one, perhaps as big as all the
> others combined.  So expecting the same amount of manpower and time
> dedication is impossible.

I agree with you, however our release process still has a gap.  We
(Intel) test commits on master, and file bugs when we find them in i965
or other components.

If those commits already have a stable tag in the commit message, they
will be shipped at a later date directly to customers, with no testing.
There is no way to blacklist broken patches in our Mesa's release
automation.

> HTH
> Emil
>
> [1] https://lists.freedesktop.org/archives/mesa-dev/2014-July/062992.html
> _______________________________________________
> mesa-stable mailing list
> mesa-stable at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable