Re: [Intel-xe] [PATCH] drm/xe/uapi: Remove DRM_IOCTL_XE_EXEC_QUEUE_SET_PROPERTY

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Wed, Dec 06, 2023 at 05:52:52PM +0000, Souza, Jose wrote:
> On Wed, 2023-12-06 at 11:48 -0600, Lucas De Marchi wrote:
> > On Wed, Dec 06, 2023 at 12:42:27PM -0500, Rodrigo Vivi wrote:
> > > On Wed, Dec 06, 2023 at 10:33:35AM -0600, Lucas De Marchi wrote:
> > > > On Wed, Dec 06, 2023 at 11:26:18AM -0500, Rodrigo Vivi wrote:
> > > > > On Wed, Dec 06, 2023 at 10:16:22AM -0600, Lucas De Marchi wrote:
> > > > > > On Wed, Dec 06, 2023 at 04:06:17PM +0000, Francois Dugast wrote:
> > > > > > > The exec_queue_set_property feature was removed in a previous
> > > > > > > commit ("drm/xe/uapi: Kill exec_queue_set_property") and is no
> > > > > > > longer usable, struct drm_xe_exec_queue_set_property does not
> > > > > > > exist anymore, so let's remove this.
> > > > > 
> > > > > my bad, sorry
> > > > > 
> > > > > > > 
> > > > > > > Signed-off-by: Francois Dugast <francois.dugast@intel.com>
> > > > > > 
> > > > > > should probably be a fixup to that patch? Anyway,
> > > > > 
> > > > > no because it is in the uapi header that gets "copied" by the UMDs
> > > > > and IGT already merged and Mesa likely... and we cannot do fixup
> > > > > on their sides.
> > > > 
> > > > wat?
> > > > 
> > > > any UMDs would just copy the header and point to the commit it came
> > > > from. It doesn't matter if we used a fixup or not.
> > > 
> > > But history wouldn't align. If they have already merged the commit with
> > > the sync pointing to a kernel commit that we do a later fixup, there
> > > would be a mismatch. Anyone looking the history in a later time would
> > > think that the UMD code was wrong.
> > > 
> > > And instead of blaming me for having missed this line in the first
> > > commit they would unfairly blame whoever authored the UMD patch
> > > 
> > > right Jose?
> > > 
> > > But I would be okay with a fixup if that is okay for Jose.
> > 
> > 
> > when a fixup is amended, it's not like the commit remains the same. You
> > will have a different hash for the new commit. Just point to the new
> > commit which has the same commit message as the previous one.
> > 
> > And we are still on top of drm-tip. All these commits will still be
> > rebased and have their hashes changed anyway.

for this reason, any patch like this I do in IGT I use the commit subject
but without any hash.

> > 
> > As long as we keep saving the tags so commits don't eventually
> > disappear from the remote, we should be fine.
> 
> I agree that for UMDs history have a fixup would be odd.
> 
> Mesa would have two commits pointing to the same commit title but with different hashes.
> 
> > 
> > Lucas De Marchi
> > 
> > > 
> > > > 
> > > > Lucas De Marchi
> > > > 
> > > > > 
> > > > > Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > 
> > > > > > 
> > > > > > Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > > > > 
> > > > > > Lucas De Marchi
> > > > > > 
> > > > > > > ---
> > > > > > > include/uapi/drm/xe_drm.h | 1 -
> > > > > > > 1 file changed, 1 deletion(-)
> > > > > > > 
> > > > > > > diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> > > > > > > index dee750385161..346007e8074d 100644
> > > > > > > --- a/include/uapi/drm/xe_drm.h
> > > > > > > +++ b/include/uapi/drm/xe_drm.h
> > > > > > > @@ -53,7 +53,6 @@ extern "C" {
> > > > > > > #define DRM_IOCTL_XE_VM_BIND			DRM_IOW(DRM_COMMAND_BASE + DRM_XE_VM_BIND, struct drm_xe_vm_bind)
> > > > > > > #define DRM_IOCTL_XE_EXEC_QUEUE_CREATE		DRM_IOWR(DRM_COMMAND_BASE + DRM_XE_EXEC_QUEUE_CREATE, struct drm_xe_exec_queue_create)
> > > > > > > #define DRM_IOCTL_XE_EXEC_QUEUE_DESTROY		DRM_IOW(DRM_COMMAND_BASE + DRM_XE_EXEC_QUEUE_DESTROY, struct drm_xe_exec_queue_destroy)
> > > > > > > -#define DRM_IOCTL_XE_EXEC_QUEUE_SET_PROPERTY	DRM_IOW(DRM_COMMAND_BASE + DRM_XE_EXEC_QUEUE_SET_PROPERTY, struct drm_xe_exec_queue_set_property)
> > > > > > > #define DRM_IOCTL_XE_EXEC_QUEUE_GET_PROPERTY	DRM_IOWR(DRM_COMMAND_BASE + DRM_XE_EXEC_QUEUE_GET_PROPERTY, struct drm_xe_exec_queue_get_property)
> > > > > > > #define DRM_IOCTL_XE_EXEC			DRM_IOW(DRM_COMMAND_BASE + DRM_XE_EXEC, struct drm_xe_exec)
> > > > > > > #define DRM_IOCTL_XE_WAIT_USER_FENCE		DRM_IOWR(DRM_COMMAND_BASE + DRM_XE_WAIT_USER_FENCE, struct drm_xe_wait_user_fence)
> > > > > > > --
> > > > > > > 2.34.1
> > > > > > > 
>