Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1566374 > unrolled thread

[PATCH] drm/ttm: Make sure BOs being swapped out are cacheable

Back to article view | Back to linux.kernel

Contents

  [PATCH] drm/ttm: Make sure BOs being swapped out are cacheable Michel Dänzer <michel@daenzer.net> - 2017-01-25 09:30 +0100
    Re: [PATCH] drm/ttm: Make sure BOs being swapped out are cacheable Michel Dänzer <michel@daenzer.net> - 2017-01-27 03:30 +0100
      Re: [PATCH] drm/ttm: Make sure BOs being swapped out are cacheable Daniel Vetter <daniel@ffwll.ch> - 2017-01-27 08:40 +0100
        Re: [PATCH] drm/ttm: Make sure BOs being swapped out are cacheable Daniel Vetter <daniel@ffwll.ch> - 2017-01-27 15:20 +0100
          Re: [PATCH] drm/ttm: Make sure BOs being swapped out are cacheable Daniel Vetter <daniel@ffwll.ch> - 2017-01-27 16:00 +0100
        Re: [PATCH] drm/ttm: Make sure BOs being swapped out are cacheable Alex Deucher <alexdeucher@gmail.com> - 2017-01-27 16:20 +0100

#1566374 — [PATCH] drm/ttm: Make sure BOs being swapped out are cacheable

From: Michel Dänzer <michel.daenzer@amd.com>

The current caching state may not be tt_cached, even though the
placement contains TTM_PL_FLAG_CACHED, because placement can contain
multiple caching flags. Trying to swap out such a BO would trip up the

	BUG_ON(ttm->caching_state != tt_cached);

in ttm_tt_swapout.

Cc: stable@vger.kernel.org
Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index d5063618efa7..86e3b233b722 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1670,7 +1670,6 @@ static int ttm_bo_swapout(struct ttm_mem_shrink *shrink)
 	struct ttm_buffer_object *bo;
 	int ret = -EBUSY;
 	int put_count;
-	uint32_t swap_placement = (TTM_PL_FLAG_CACHED | TTM_PL_FLAG_SYSTEM);
 
 	spin_lock(&glob->lru_lock);
 	list_for_each_entry(bo, &glob->swap_lru, swap) {
@@ -1701,7 +1700,8 @@ static int ttm_bo_swapout(struct ttm_mem_shrink *shrink)
 	 * Move to system cached
 	 */
 
-	if ((bo->mem.placement & swap_placement) != swap_placement) {
+	if (bo->mem.mem_type != TTM_PL_SYSTEM ||
+	    bo->ttm->caching_state != tt_cached) {
 		struct ttm_mem_reg evict_mem;
 
 		evict_mem = bo->mem;
-- 
2.11.0

[toc] | [next] | [standalone]

#1567872

On 26/01/17 09:46 AM, Sinclair Yeh wrote:
> On Wed, Jan 25, 2017 at 10:49:33AM +0100, Christian König wrote:
>> Am 25.01.2017 um 10:25 schrieb Thomas Hellstrom:
>>> On 01/25/2017 09:21 AM, Michel Dänzer wrote:
>>>> From: Michel Dänzer <michel.daenzer@amd.com>
>>>>
>>>> The current caching state may not be tt_cached, even though the
>>>> placement contains TTM_PL_FLAG_CACHED, because placement can contain
>>>> multiple caching flags. Trying to swap out such a BO would trip up the
>>>>
>>>> 	BUG_ON(ttm->caching_state != tt_cached);
>>>>
>>>> in ttm_tt_swapout.
>>>>
>>>> Cc: stable@vger.kernel.org
>>>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
>>> Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
>>
>> Reviewed-by: Christian König <christian.koenig@amd.com>.
> 
> Reviewed-by: Sinclair Yeh <syeh@vmware.com>

Thanks for the reviews! Via which tree should we merge this?


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer

[toc] | [prev] | [next] | [standalone]

#1567916

On Fri, Jan 27, 2017 at 07:23:58AM +0100, Thomas Hellstrom wrote:
> On 01/27/2017 03:29 AM, Michel Dänzer wrote:
> > On 26/01/17 09:46 AM, Sinclair Yeh wrote:
> >> On Wed, Jan 25, 2017 at 10:49:33AM +0100, Christian König wrote:
> >>> Am 25.01.2017 um 10:25 schrieb Thomas Hellstrom:
> >>>> On 01/25/2017 09:21 AM, Michel Dänzer wrote:
> >>>>> From: Michel Dänzer <michel.daenzer@amd.com>
> >>>>>
> >>>>> The current caching state may not be tt_cached, even though the
> >>>>> placement contains TTM_PL_FLAG_CACHED, because placement can contain
> >>>>> multiple caching flags. Trying to swap out such a BO would trip up the
> >>>>>
> >>>>> 	BUG_ON(ttm->caching_state != tt_cached);
> >>>>>
> >>>>> in ttm_tt_swapout.
> >>>>>
> >>>>> Cc: stable@vger.kernel.org
> >>>>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
> >>>> Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
> >>> Reviewed-by: Christian König <christian.koenig@amd.com>.
> >> Reviewed-by: Sinclair Yeh <syeh@vmware.com>
> > Thanks for the reviews! Via which tree should we merge this?
> >
> >
> I don't maintain a TTM tree any longer. Let's check with Daniel if he
> can merge it through drm-misc.

I'm trying very hard not to get volunteered for ttm maintainer :-)
Nominally Alex&Christian have drm-misc commit rights, but they haven't
used them yet. But I think merging through drm-misc would make sense,
there's regular pull request trains for both -next and -fixes. Or merge
through the amd tree with Dave's ack, but I'd really like to get amd folks
into the drm-misc group ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[toc] | [prev] | [next] | [standalone]

#1568400

On Fri, Jan 27, 2017 at 09:22:47AM +0100, Christian König wrote:
> Am 27.01.2017 um 08:30 schrieb Daniel Vetter:
> > On Fri, Jan 27, 2017 at 07:23:58AM +0100, Thomas Hellstrom wrote:
> > > On 01/27/2017 03:29 AM, Michel Dänzer wrote:
> > > > On 26/01/17 09:46 AM, Sinclair Yeh wrote:
> > > > > On Wed, Jan 25, 2017 at 10:49:33AM +0100, Christian König wrote:
> > > > > > Am 25.01.2017 um 10:25 schrieb Thomas Hellstrom:
> > > > > > > On 01/25/2017 09:21 AM, Michel Dänzer wrote:
> > > > > > > > From: Michel Dänzer <michel.daenzer@amd.com>
> > > > > > > > 
> > > > > > > > The current caching state may not be tt_cached, even though the
> > > > > > > > placement contains TTM_PL_FLAG_CACHED, because placement can contain
> > > > > > > > multiple caching flags. Trying to swap out such a BO would trip up the
> > > > > > > > 
> > > > > > > > 	BUG_ON(ttm->caching_state != tt_cached);
> > > > > > > > 
> > > > > > > > in ttm_tt_swapout.
> > > > > > > > 
> > > > > > > > Cc: stable@vger.kernel.org
> > > > > > > > Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
> > > > > > > Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
> > > > > > Reviewed-by: Christian König <christian.koenig@amd.com>.
> > > > > Reviewed-by: Sinclair Yeh <syeh@vmware.com>
> > > > Thanks for the reviews! Via which tree should we merge this?
> > > > 
> > > > 
> > > I don't maintain a TTM tree any longer. Let's check with Daniel if he
> > > can merge it through drm-misc.
> > I'm trying very hard not to get volunteered for ttm maintainer :-)
> 
> Yeah, ok I volunteer. Wanted to take that over for a while anyway.

I guess you didn't use the dim magic to push it? The test integration tree
isn't rebuild ... Quickstart for the tooling:

https://01.org/linuxgraphics/gfx-docs/maintainer-tools/dim.html

-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[toc] | [prev] | [next] | [standalone]

#1568421

On Fri, Jan 27, 2017 at 03:43:18PM +0100, Christian König wrote:
> Am 27.01.2017 um 15:12 schrieb Daniel Vetter:
> > On Fri, Jan 27, 2017 at 09:22:47AM +0100, Christian König wrote:
> > > Am 27.01.2017 um 08:30 schrieb Daniel Vetter:
> > > > On Fri, Jan 27, 2017 at 07:23:58AM +0100, Thomas Hellstrom wrote:
> > > > > On 01/27/2017 03:29 AM, Michel Dänzer wrote:
> > > > > > On 26/01/17 09:46 AM, Sinclair Yeh wrote:
> > > > > > > On Wed, Jan 25, 2017 at 10:49:33AM +0100, Christian König wrote:
> > > > > > > > Am 25.01.2017 um 10:25 schrieb Thomas Hellstrom:
> > > > > > > > > On 01/25/2017 09:21 AM, Michel Dänzer wrote:
> > > > > > > > > > From: Michel Dänzer <michel.daenzer@amd.com>
> > > > > > > > > > 
> > > > > > > > > > The current caching state may not be tt_cached, even though the
> > > > > > > > > > placement contains TTM_PL_FLAG_CACHED, because placement can contain
> > > > > > > > > > multiple caching flags. Trying to swap out such a BO would trip up the
> > > > > > > > > > 
> > > > > > > > > > 	BUG_ON(ttm->caching_state != tt_cached);
> > > > > > > > > > 
> > > > > > > > > > in ttm_tt_swapout.
> > > > > > > > > > 
> > > > > > > > > > Cc: stable@vger.kernel.org
> > > > > > > > > > Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
> > > > > > > > > Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
> > > > > > > > Reviewed-by: Christian König <christian.koenig@amd.com>.
> > > > > > > Reviewed-by: Sinclair Yeh <syeh@vmware.com>
> > > > > > Thanks for the reviews! Via which tree should we merge this?
> > > > > > 
> > > > > > 
> > > > > I don't maintain a TTM tree any longer. Let's check with Daniel if he
> > > > > can merge it through drm-misc.
> > > > I'm trying very hard not to get volunteered for ttm maintainer :-)
> > > Yeah, ok I volunteer. Wanted to take that over for a while anyway.
> > I guess you didn't use the dim magic to push it? The test integration tree
> > isn't rebuild ... Quickstart for the tooling:
> 
> Autsch! Do I have to use that or can I just go with my usual tool set?
> 
> Would probably be a pain to set this up everywhere here.

We use it both for convience (e.g. it auto-adds the sob if need, and the
patchwork link by default), and to catch mistakes (e.g. it checks that
you're not pushing patches outside of the scope of drm-misc or i915 to the
wrong tree). For drm-misc specifically there's also the 3 defconfigs
you're supposed to build test before pushing (unfornately not scripted
since distros can't even agree on what cross-compiler prefixes work for a
given platform). And since 1 month ago we have an arm-soc ttm-based
driver, so doing that for ttm patches is kinda a good idea too :-)

But right now the main thing is this integration tree magic to regenerate
drm-tip, and that's also what intel CI and kernelci from collabora beat
on. Long term I'd love to push all this onto the server-side (github makes
scipting a lot of this dead easy). But we're still stuck with patches
scribbled on stones with emailed patches and all that, so client-side
scripts are kinda needed.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[toc] | [prev] | [next] | [standalone]

#1568426

On Fri, Jan 27, 2017 at 3:22 AM, Christian König
<christian.koenig@amd.com> wrote:
> Am 27.01.2017 um 08:30 schrieb Daniel Vetter:
>>
>> On Fri, Jan 27, 2017 at 07:23:58AM +0100, Thomas Hellstrom wrote:
>>>
>>> On 01/27/2017 03:29 AM, Michel Dänzer wrote:
>>>>
>>>> On 26/01/17 09:46 AM, Sinclair Yeh wrote:
>>>>>
>>>>> On Wed, Jan 25, 2017 at 10:49:33AM +0100, Christian König wrote:
>>>>>>
>>>>>> Am 25.01.2017 um 10:25 schrieb Thomas Hellstrom:
>>>>>>>
>>>>>>> On 01/25/2017 09:21 AM, Michel Dänzer wrote:
>>>>>>>>
>>>>>>>> From: Michel Dänzer <michel.daenzer@amd.com>
>>>>>>>>
>>>>>>>> The current caching state may not be tt_cached, even though the
>>>>>>>> placement contains TTM_PL_FLAG_CACHED, because placement can contain
>>>>>>>> multiple caching flags. Trying to swap out such a BO would trip up
>>>>>>>> the
>>>>>>>>
>>>>>>>>         BUG_ON(ttm->caching_state != tt_cached);
>>>>>>>>
>>>>>>>> in ttm_tt_swapout.
>>>>>>>>
>>>>>>>> Cc: stable@vger.kernel.org
>>>>>>>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
>>>>>>>
>>>>>>> Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
>>>>>>
>>>>>> Reviewed-by: Christian König <christian.koenig@amd.com>.
>>>>>
>>>>> Reviewed-by: Sinclair Yeh <syeh@vmware.com>
>>>>
>>>> Thanks for the reviews! Via which tree should we merge this?
>>>>
>>>>
>>> I don't maintain a TTM tree any longer. Let's check with Daniel if he
>>> can merge it through drm-misc.
>>
>> I'm trying very hard not to get volunteered for ttm maintainer :-)
>
>
> Yeah, ok I volunteer. Wanted to take that over for a while anyway.
>
>> Nominally Alex&Christian have drm-misc commit rights, but they haven't
>> used them yet. But I think merging through drm-misc would make sense,
>> there's regular pull request trains for both -next and -fixes.
>
>
> Completely agree on merging it through drm-misc. Going to give my push
> rights a try today.
>
>> Or merge through the amd tree with Dave's ack, but I'd really like to get
>> amd folks
>> into the drm-misc group ...
>
>
> I've got a few more already reviewed TTM changes which are currently waiting
> to be pushed upstream where amdgpu has dependencies on.
>
> Going to sync with Alex so he sends his pull requests with those changes to
> Dave after the merge of the depending changes.
>
> If you want to object that just merging through the AMD tree would be
> simpler, I agree but I actually want to do this exercise at least once :)

I was thinking of including this in my -fixes pull next week, but I
don't have a strong preference either way.  The other ttm changes are
pretty intertwined with amdgpu changes so I think it would be easier
to take them in via the amdgpu tree.  In the future we can adjust
that.

Alex

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web