[PATCH] drm/xe: Fix build error in xe_ggtt.c

Lucas De Marchi lucas.demarchi at intel.com
Wed Feb 28 18:49:05 UTC 2024


On Wed, Feb 28, 2024 at 03:47:13PM +0100, Thomas Hellström wrote:
>On Mon, 2024-02-26 at 15:40 +0000, Matthew Brost wrote:
>> On Mon, Feb 26, 2024 at 10:05:58AM +0100, Thomas Hellström wrote:
>> > On Sat, 2024-02-24 at 16:14 -0800, Matthew Brost wrote:
>> > > Need to include io-64-nonatomic-lo-hi.h for writeq function.
>> >
>> > As I understand it, the choice of header here determines the dword
>> > write order on 32-bit systems that don't have an atomic writeq(),
>> >
>> > So is writing the low dword first the correct order in this case?
>> > Perhaps add a motivation in the commit message?
>> >
>>
>> "Cleanup some layering in GGTT" removed xe_mmio.h from xe_gt.c and
>> that
>> file includes linux/io-64-nonatomic-lo-hi.h. Perhaps it is better
>> just
>> to include xe_mmio.h again in xe_gt.c?
>
>I think it then makes sense to use your original patch, so that it's
>easier to follow for each subsystem what ordering is used.

what original patch, I only find this one in the mailing list.
and it seems correct to me, following the "include what you use"
approach.

Also see some more details in commit 9a6e6c14bfde ("drm/xe/mmio: Use
non-atomic writeq/readq variant for 32b"):

	writeq() and readq() and other functions working on 64 bit variables
	are not provided by 32b arch. For that it's needed to choose between
	linux/io-64-nonatomic-hi-lo.h and linux/io-64-nonatomic-lo-hi.h,
	spliting the read/write in 2 accesses. For xe driver, it doesn't matter
	much, so just choose one and include in xe_mmio.h.

Aside from the "Fixes:" trailer, Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>

thanks
Lucas De Marchi

>
>Also note Jani's comment. We should use "dim fixes <hash>" to get the
>fixes tag correct.
>
>Thanks,
>Thomas
>
>
>>
>> Matt
>>
>> > /Thomas
>> >
>> >
>> > >
>> > > Fixes: 3121fed0c51b drm/xe: ("Cleanup some layering in GGTT")
>> > > Reported-by: kernel test robot <lkp at intel.com>
>> > > Closes:
>> > > https://lore.kernel.org/oe-kbuild-all/202402241903.R5J8hKVI-lkp@intel.com/
>> > > Signed-off-by: Matthew Brost <matthew.brost at intel.com>
>> > > ---
>> > >  drivers/gpu/drm/xe/xe_ggtt.c | 1 +
>> > >  1 file changed, 1 insertion(+)
>> > >
>> > > diff --git a/drivers/gpu/drm/xe/xe_ggtt.c
>> > > b/drivers/gpu/drm/xe/xe_ggtt.c
>> > > index 5d46958e3144..717d0e76277a 100644
>> > > --- a/drivers/gpu/drm/xe/xe_ggtt.c
>> > > +++ b/drivers/gpu/drm/xe/xe_ggtt.c
>> > > @@ -5,6 +5,7 @@
>> > >  
>> > >  #include "xe_ggtt.h"
>> > >  
>> > > +#include <linux/io-64-nonatomic-lo-hi.h>
>> > >  #include <linux/sizes.h>
>> > >  
>> > >  #include <drm/drm_managed.h>
>> >
>


More information about the Intel-xe mailing list