Hi Thomas,

I cannot drop the older DRI1 based uAPI calls.
This is because include/uapi/drm/via_drm.h needs to retain backward 
compatibility with the existing OpenChrome DDX's XvMC library (it gets compiled 
when OpenChrome DDX is built) and likely with the existing DDX Xv code as well.
If I remove the DRI1 based uAPI calls, the XvMC library will not get compiled 
(compile error will occur since the XvMC library assumes the presence of DRI1 
based uAPI), and I assume the same for the DDX Xv code (I cannot even reach 
here since the XvMC library is compiled first).
Although the v3 patch does not contain it, v4 patch will utilize 
drm_invalid_op() for the discontinued (not deprecated since OpenChrome DRM does 
not support the older DRI1 based uAPI at all) DRI1 based uAPI.

https://cgit.freedesktop.org/openchrome/drm-openchrome/commit/?h=drm-next-5.20&id=16b3d68f95c9ccd15b7a3310e5d752fabbc76518

drm_invalid_op() is related to drm_ioctl.c, and is meant for legacy DRMs like 
Radeon, i915, etc.
Since OpenChrome DRM is not a clean sheet design (related to VIA DRM to some 
extent), I will use this function for properly handling discontinued legacy 
uAPI calls.
I hope this explanation / reasoning is okay with you.

Regards,

Kevin Brace
Brace Computer Laboratory blog
https://bracecomputerlab.com

> Sent: Tuesday, July 26, 2022 at 11:24 AM
> From: "Thomas Zimmermann" <tzimmerm...@suse.de>
> To: "Kevin Brace" <kevinbr...@gmx.com>, dri-devel@lists.freedesktop.org
> Cc: "Kevin Brace" <kevinbr...@bracecomputerlab.com>
> Subject: Re: [PATCH v3 26/32] drm/via: Add via_drm.h
>
> Hi
> 
> Am 26.07.22 um 01:53 schrieb Kevin Brace:
> > From: Kevin Brace <kevinbr...@bracecomputerlab.com>
> > 
> > Changed the uAPI.
> > 
> > Signed-off-by: Kevin Brace <kevinbr...@bracecomputerlab.com>
> > ---
> >   include/uapi/drm/via_drm.h | 35 +++++++++++++++++++++++++++++++----
> >   1 file changed, 31 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/uapi/drm/via_drm.h b/include/uapi/drm/via_drm.h
> > index a1e125d42208..e9da45ce130a 100644
> > --- a/include/uapi/drm/via_drm.h
> > +++ b/include/uapi/drm/via_drm.h
> > @@ -1,4 +1,5 @@
> >   /*
> > + * Copyright © 2020 Kevin Brace
> >    * Copyright 1998-2003 VIA Technologies, Inc. All Rights Reserved.
> >    * Copyright 2001-2003 S3 Graphics, Inc. All Rights Reserved.
> >    *
> > @@ -16,10 +17,10 @@
> >    * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
> > EXPRESS OR
> >    * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
> > MERCHANTABILITY,
> >    * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT 
> > SHALL
> > - * VIA, S3 GRAPHICS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES 
> > OR
> > - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR 
> > OTHER
> > - * DEALINGS IN THE SOFTWARE.
> > + * THE AUTHORS, COPYRIGHT HOLDERS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY
> > + * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, 
> > TORT
> > + * OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR
> > + * THE USE OR OTHER DEALINGS IN THE SOFTWARE.
> >    */
> >   #ifndef _VIA_DRM_H_
> >   #define _VIA_DRM_H_
> > @@ -81,6 +82,11 @@ extern "C" {
> >   #define DRM_VIA_DMA_BLIT        0x0e
> >   #define DRM_VIA_BLIT_SYNC       0x0f
> > 
> > +#define    DRM_VIA_GEM_CREATE      0x10
> > +#define    DRM_VIA_GEM_MAP         0x11
> > +#define    DRM_VIA_GEM_UNMAP       0x12
> 
> This looks a lot like ioctl ops for using accelerated HW buffers. But 
> the patch is near the end of the series and you said in the series' 
> cover letter that there's no acceleration. I suspect that these 
> constants are currently unused?  If so, please drop the patch from the 
> series. If can be merged later when something really depends on it.
> 
> Best regards
> Thomas
> 
> > +
> > +
> >   #define DRM_IOCTL_VIA_ALLOCMEM      DRM_IOWR(DRM_COMMAND_BASE + 
> > DRM_VIA_ALLOCMEM, drm_via_mem_t)
> >   #define DRM_IOCTL_VIA_FREEMEM       DRM_IOW( DRM_COMMAND_BASE + 
> > DRM_VIA_FREEMEM, drm_via_mem_t)
> >   #define DRM_IOCTL_VIA_AGP_INIT      DRM_IOWR(DRM_COMMAND_BASE + 
> > DRM_VIA_AGP_INIT, drm_via_agp_t)
> > @@ -97,6 +103,10 @@ extern "C" {
> >   #define DRM_IOCTL_VIA_DMA_BLIT    DRM_IOW(DRM_COMMAND_BASE + 
> > DRM_VIA_DMA_BLIT, drm_via_dmablit_t)
> >   #define DRM_IOCTL_VIA_BLIT_SYNC   DRM_IOW(DRM_COMMAND_BASE + 
> > DRM_VIA_BLIT_SYNC, drm_via_blitsync_t)
> > 
> > +#define    DRM_IOCTL_VIA_GEM_CREATE        DRM_IOWR(DRM_COMMAND_BASE + 
> > DRM_VIA_GEM_CREATE, struct drm_via_gem_create)
> > +#define    DRM_IOCTL_VIA_GEM_MAP           DRM_IOWR(DRM_COMMAND_BASE + 
> > DRM_VIA_GEM_MAP, struct drm_via_gem_map)
> > +#define    DRM_IOCTL_VIA_GEM_UNMAP         DRM_IOR(DRM_COMMAND_BASE + 
> > DRM_VIA_GEM_UNMAP, struct drm_via_gem_unmap)
> > +
> >   /* Indices into buf.Setup where various bits of state are mirrored per
> >    * context and per buffer.  These can be fired at the card as a unit,
> >    * or in a piecewise fashion as required.
> > @@ -275,6 +285,23 @@ typedef struct drm_via_dmablit {
> >     drm_via_blitsync_t sync;
> >   } drm_via_dmablit_t;
> > 
> > +struct drm_via_gem_create {
> > +   uint64_t size;
> > +   uint32_t alignment;
> > +   uint32_t domain;
> > +   uint32_t handle;
> > +   uint64_t offset;
> > +};
> > +
> > +struct drm_via_gem_map {
> > +   uint32_t handle;
> > +   uint64_t map_offset;
> > +};
> > +
> > +struct drm_via_gem_unmap {
> > +   uint32_t handle;
> > +};
> > +
> >   #if defined(__cplusplus)
> >   }
> >   #endif
> > --
> > 2.35.1
> > 
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Ivo Totev
>

Reply via email to