Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 30 Jul 2019 15:08:52 +0100
From:   Emil Velikov <emil.l.velikov@gmail.com>
To:     Jan Sebastian =?iso-8859-1?Q?G=F6tte?= <linux@jaseg.net>
Cc:     dri-devel@lists.freedesktop.org, David Airlie <airlied@linux.ie>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 5/6] drm: uapi: add gdepaper uapi header
Message-ID: <20190730140852.GB12424@arch-x1c3>
References: <3c8fccc9-63f7-18bb-dcb5-dcd0b9e151d2@jaseg.net>
 <0e22c86a-3998-c2fd-cb14-203df547eb5c@jaseg.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <0e22c86a-3998-c2fd-cb14-203df547eb5c@jaseg.net>
User-Agent: Mutt/1.11.4 (2019-03-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Jan,

On 2019/07/30, Jan Sebastian G?tte wrote:
> Signed-off-by: Jan Sebastian G?tte <linux@jaseg.net>
> ---
>  include/uapi/drm/gdepaper_drm.h | 62 +++++++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
>  create mode 100644 include/uapi/drm/gdepaper_drm.h
> 
> diff --git a/include/uapi/drm/gdepaper_drm.h b/include/uapi/drm/gdepaper_drm.h
> new file mode 100644
> index 000000000000..84ff6429bef5
> --- /dev/null
> +++ b/include/uapi/drm/gdepaper_drm.h
> @@ -0,0 +1,62 @@
> +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
> +/* gdepaper_drm.h
> + *
> + * Copyright (c) 2019 Jan Sebastian G?tte
> + */
> +
I'm glad to see more devices using upstream KMS interface. Usually
custom UAPI should not be needed for such devices.

Can you elaborate why this is required? Is there an open-source
userspace that uses these?


> +#ifndef _UAPI_GDEPAPER_DRM_H_
> +#define _UAPI_GDEPAPER_DRM_H_
> +
> +#include "drm.h"
> +
> +#if defined(__cplusplus)
> +extern "C" {
> +#endif
> +
> +enum drm_gdepaper_vghl_lv {
> +	DRM_GDEP_PWR_VGHL_16V = 0,
> +	DRM_GDEP_PWR_VGHL_15V = 1,
> +	DRM_GDEP_PWR_VGHL_14V = 2,
> +	DRM_GDEP_PWR_VGHL_13V = 3,
> +};
> +
> +struct gdepaper_refresh_params {
> +	enum drm_gdepaper_vghl_lv vg_lv; /* gate voltage level */
From my experience, kernel drivers aim to avoid exposing voltage control
to userspace. AFAICT exceptions are present, but generally it's a no-go.


> +	__u32 vcom_sel; /* VCOM select bit according to datasheet */
> +	__s32 vdh_bw_mv; /* drive high level, b/w pixel (mV) */
> +	__s32 vdh_col_mv; /* drive high level, red/yellow pixel (mV) */
> +	__s32 vdl_mv; /* drive low level (mV) */
> +	__s32 vcom_dc_mv;
> +	__u32 vcom_data_ivl_hsync; /* data ivl len in hsync periods */
> +	__u32 border_data_sel; /* "vbd" in datasheet */
> +	__u32 data_polarity; /* "ddx" in datasheet */
These properties look like they should live in the device-tree bindings.

> +	__u32 use_otp_luts_flag; /* Use OTP LUTs */
> +	__u8 lut_vcom_dc[44];
> +	__u8 lut_ww[42];
> +	__u8 lut_bw[42];
> +	__u8 lut_bb[42];
> +	__u8 lut_wb[42];
There is UAPI to manage LUT (or was it WIP with patches on the list) via
the atomic API. Is that not flexible enough for your needs?

> +};
> +
> +/* Force a full display refresh */
> +#define DRM_GDEPAPER_FORCE_FULL_REFRESH		0x00
> +#define DRM_GDEPAPER_GET_REFRESH_PARAMS		0x01
> +#define DRM_GDEPAPER_SET_REFRESH_PARAMS		0x02
> +#define DRM_GDEPAPER_SET_PARTIAL_UPDATE_EN	0x03
> +
> +#define DRM_IOCTL_GDEPAPER_FORCE_FULL_REFRESH \
> +	DRM_IO(DRM_COMMAND_BASE + DRM_GDEPAPER_FORCE_FULL_REFRESH)
> +#define DRM_IOCTL_GDEPAPER_GET_REFRESH_PARAMS \
> +	DRM_IOR(DRM_COMMAND_BASE + DRM_GDEPAPER_GET_REFRESH_PARAMS, \
> +	struct gdepaper_refresh_params)
> +#define DRM_IOCTL_GDEPAPER_SET_REFRESH_PARAMS \
> +	DRM_IOR(DRM_COMMAND_BASE + DRM_GDEPAPER_SET_REFRESH_PARAMS, \
> +	struct gdepaper_refresh_params)
> +#define DRM_IOCTL_GDEPAPER_SET_PARTIAL_UPDATE_EN \
> +	DRM_IOR(DRM_COMMAND_BASE + DRM_GDEPAPER_SET_PARTIAL_UPDATE_EN, __u32)
> +
Similarly to the LUT above, the atomic UAPI has support for partial
updates. The property is called FB_DAMAGE_CLIPS and there is an example
in weston how to use it see 009b3cfa6f16bb361eb54efcc7435bfede4524bc.

HTH
Emil