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

[Juan A. Suarez Romero]

On Wed, 2018-04-04 at 09:40 -0700, 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?
> 
> Release managers: is there a protocol for how this specification should
> be parsed, when considering a patch for stable?
> 

In my case, if a patch is nominated, then it is applied to the proper stable
branch. I understand that while the patch probably fixes a commit, it makes
sense per se as a valid independent patch.

But I think we should modify our script to warn us in this situation and just
clarify with the author about the intention.


	J.A.


> > ---
> >  src/gallium/drivers/radeon/radeon_vce.c        |  1 -
> >  src/gallium/drivers/radeon/radeon_vce_40_2_2.c |  2 ++
> >  src/gallium/drivers/radeon/radeon_vce_52.c     | 18 ++++++++++--------
> >  3 files changed, 12 insertions(+), 9 deletions(-)
> > 
> > diff --git a/src/gallium/drivers/radeon/radeon_vce.c b/src/gallium/drivers/radeon/radeon_vce.c
> > index 427bf01ed8..c84103e0ac 100644
> > --- a/src/gallium/drivers/radeon/radeon_vce.c
> > +++ b/src/gallium/drivers/radeon/radeon_vce.c
> > @@ -247,7 +247,6 @@ static void rvce_destroy(struct pipe_video_codec *encoder)
> >  		enc->fb = &fb;
> >  		enc->session(enc);
> >  		enc->destroy(enc);
> > -		enc->feedback(enc);
> >  		flush(enc);
> >  		si_vid_destroy_buffer(&fb);
> >  	}
> > diff --git a/src/gallium/drivers/radeon/radeon_vce_40_2_2.c b/src/gallium/drivers/radeon/radeon_vce_40_2_2.c
> > index f1db47d4bd..04e9d7f5e1 100644
> > --- a/src/gallium/drivers/radeon/radeon_vce_40_2_2.c
> > +++ b/src/gallium/drivers/radeon/radeon_vce_40_2_2.c
> > @@ -421,6 +421,8 @@ static void destroy(struct rvce_encoder *enc)
> >  {
> >  	enc->task_info(enc, 0x00000001, 0, 0, 0);
> >  
> > +	feedback(enc);
> > +
> >  	RVCE_BEGIN(0x02000001); // destroy
> >  	RVCE_END();
> >  }
> > diff --git a/src/gallium/drivers/radeon/radeon_vce_52.c b/src/gallium/drivers/radeon/radeon_vce_52.c
> > index a941c476f6..421539c4bd 100644
> > --- a/src/gallium/drivers/radeon/radeon_vce_52.c
> > +++ b/src/gallium/drivers/radeon/radeon_vce_52.c
> > @@ -458,14 +458,6 @@ static void config_extension(struct rvce_encoder *enc)
> >  	RVCE_END();
> >  }
> >  
> > -static void destroy(struct rvce_encoder *enc)
> > -{
> > -	enc->task_info(enc, 0x00000001, 0, 0, 0);
> > -
> > -	RVCE_BEGIN(0x02000001); // destroy
> > -	RVCE_END();
> > -}
> > -
> >  static void feedback(struct rvce_encoder *enc)
> >  {
> >  	RVCE_BEGIN(0x05000005); // feedback buffer
> > @@ -474,6 +466,16 @@ static void feedback(struct rvce_encoder *enc)
> >  	RVCE_END();
> >  }
> >  
> > +static void destroy(struct rvce_encoder *enc)
> > +{
> > +	enc->task_info(enc, 0x00000001, 0, 0, 0);
> > +
> > +	feedback(enc);
> > +
> > +	RVCE_BEGIN(0x02000001); // destroy
> > +	RVCE_END();
> > +}
> > +
> >  static void motion_estimation(struct rvce_encoder *enc)
> >  {
> >  	RVCE_BEGIN(0x04000007); // motion estimation
> > -- 
> > 2.14.1
> > 
> > _______________________________________________
> > mesa-stable mailing list
> > mesa-stable at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-stable
> 
>