From: Sam Ravnborg <sam@ravnborg.org>
To: Kevin Brace <kevinbrace@gmx.com>
Cc: Kevin Brace <kevinbrace@bracecomputerlab.com>,
dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v3 26/32] drm/via: Add via_drm.h
Date: Tue, 26 Jul 2022 19:41:49 +0200 [thread overview]
Message-ID: <YuAnXcz0ImO+cAHu@ravnborg.org> (raw)
In-Reply-To: <20220725235359.20516-7-kevinbrace@gmx.com>
Hi Kevin.
On Mon, Jul 25, 2022 at 04:53:53PM -0700, Kevin Brace wrote:
> From: Kevin Brace <kevinbrace@bracecomputerlab.com>
>
> Changed the uAPI.
>
> Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
It would be great to have the new extensions to the UAPI documented
using kernel-doc.
As an example see: vc4_drm.h
There are a lot of UAPI that is missing documentation, but if we do not
add it for new UAPI then this situation never improves.
Please use __u32, __u64 like you see in other drm UAPI files.
PS. If you reply to this mail, then please keep the full mail as
usually my mails to Kevin bounces (something with STARTTLS).
Sam
> ---
> 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.
> */
Do not mix license changes with other changes - and use SPDX tag if
possible for the updated license.
See other drm UAPI files for examples.
> #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
> +
Use the same alignment as the previous lines.
> +
Drop extra empty line.
> #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)
> +
Use same alignment as previous lines.
> /* 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;
> +};
I do not know if this is relevant, but adding a 64 bit parameter
(offset) that is only 32 bit aligned looks like trouble to me.
I hope others that knows this better can comment here.
> +
> +struct drm_via_gem_map {
> + uint32_t handle;
> + uint64_t map_offset;
> +};
Same here with the alignment.
If drm_via_gem_create.offset and drm_via_gem_map.map_offset is the same
the field should have the same name - to make the API easier to use.
> +
> +struct drm_via_gem_unmap {
> + uint32_t handle;
> +};
> +
> #if defined(__cplusplus)
> }
> #endif
> --
> 2.35.1
next prev parent reply other threads:[~2022-07-26 17:41 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-25 23:53 [PATCH v3 20/32] drm/via: Add via_pll.c Kevin Brace
2022-07-25 23:53 ` [PATCH v3 21/32] drm/via: Add via_pm.c Kevin Brace
2022-07-25 23:53 ` [PATCH v3 22/32] drm/via: Add via_sii164.c Kevin Brace
2022-07-25 23:53 ` [PATCH v3 23/32] drm/via: Add via_tmds.c Kevin Brace
2022-07-25 23:53 ` [PATCH v3 24/32] drm/via: Add via_ttm.c Kevin Brace
2022-07-25 23:53 ` [PATCH v3 25/32] drm/via: Add via_vt1632.c Kevin Brace
2022-07-25 23:53 ` [PATCH v3 26/32] drm/via: Add via_drm.h Kevin Brace
2022-07-26 17:41 ` Sam Ravnborg [this message]
2022-07-26 18:18 ` Thomas Zimmermann
2022-07-26 20:20 ` Dave Airlie
2022-07-27 7:36 ` Thomas Zimmermann
2022-07-29 1:06 ` Kevin Brace
2022-07-31 2:14 ` Kevin Brace
2022-07-26 18:24 ` Thomas Zimmermann
2022-07-30 22:48 ` Kevin Brace
2022-08-01 14:40 ` Sam Ravnborg
2022-08-02 0:10 ` Kevin Brace
2022-08-02 11:09 ` Sam Ravnborg
2022-08-02 18:12 ` Kevin Brace
2022-08-01 18:49 ` Thomas Zimmermann
2022-08-02 3:45 ` Kevin Brace
2022-08-02 11:38 ` Thomas Zimmermann
2022-08-03 1:49 ` Kevin Brace
2022-08-04 12:20 ` Thomas Zimmermann
2022-08-04 15:09 ` Sam Ravnborg
2022-07-25 23:53 ` [PATCH v3 27/32] drm/via: Add new VIA Technologies Chrome supporting PCI IDs to DRM Kevin Brace
2022-07-25 23:53 ` [PATCH v3 28/32] drm/via: Zero out chip type field Kevin Brace
2022-07-25 23:53 ` [PATCH v3 29/32] drm/via: Add new VIA Technologies PCI device IDs related to graphics Kevin Brace
2022-07-25 23:53 ` [PATCH v3 30/32] drm/via: Add Kconfig Kevin Brace
2022-07-26 7:28 ` Thomas Zimmermann
2022-07-31 1:49 ` Kevin Brace
2022-08-01 10:36 ` Thomas Zimmermann
2022-07-25 23:53 ` [PATCH v3 31/32] drm/via: Add Makefile Kevin Brace
2022-07-26 7:29 ` Thomas Zimmermann
2022-07-31 1:55 ` Kevin Brace
2022-07-25 23:53 ` [PATCH v3 32/32] drm/via: Modify DRM core to be able to build OpenChrome DRM Kevin Brace
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YuAnXcz0ImO+cAHu@ravnborg.org \
--to=sam@ravnborg.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=kevinbrace@bracecomputerlab.com \
--cc=kevinbrace@gmx.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).