Date:   Fri, 7 Jan 2022 11:09:39 -0800
From:   Benson Leung <bleung@google.com>
To:     Rajat Jain <rajatja@google.com>
Cc:     Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Maxime Ripard <mripard@kernel.org>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        Hans de Goede <hdegoede@redhat.com>,
        David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>,
        Benson Leung <bleung@chromium.org>,
        Henrique de Moraes Holschuh <hmh@hmh.eng.br>,
        Mark Gross <markgross@kernel.org>,
        linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
        ibm-acpi-devel@lists.sourceforge.net,
        platform-driver-x86@vger.kernel.org, gwendal@google.com,
        seanpaul@google.com, marcheu@google.com, dtor@google.com,
        Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        rajatxjain@gmail.com
Subject: Re: [PATCH v5 2/3] platform/chrome: Add driver for ChromeOS
 privacy-screen
Message-ID: <YdiP8zPHgM5XAvsN@google.com>
References: <20220107190208.95479-1-rajatja@google.com>
 <20220107190208.95479-2-rajatja@google.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="Qn5xF3c+Hpu80EQa"
Content-Disposition: inline
In-Reply-To: <20220107190208.95479-2-rajatja@google.com>
Precedence: bulk


--Qn5xF3c+Hpu80EQa
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi Rajat,


On Fri, Jan 07, 2022 at 11:02:07AM -0800, Rajat Jain wrote:
> This adds the ACPI driver for the ChromeOS privacy screen that is
> present on some chromeos devices.
>=20
> Note that ideally, we'd want this privacy screen driver to be probed
> BEFORE the drm probe in order to avoid a drm probe deferral:
> https://hansdegoede.livejournal.com/25948.html
>=20
> In practise, I found that ACPI drivers are bound to their devices AFTER
> the drm probe on chromebooks. So on chromebooks with privacy-screen,
> this patch along with the other one in this series results in a probe
> deferral of about 250ms for i915 driver. However, it did not result in
> any user noticeable delay of splash screen in my personal experience.
>=20
> In future if this probe deferral turns out to be an issue, we can
> consider turning this ACPI driver into something that is probed
> earlier than the drm drivers.
>=20
> Signed-off-by: Rajat Jain <rajatja@google.com>
> Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Acked-By: Benson Leung <bleung@chromium.org>

Hi Hans,

Could you take this patch? This new driver in platform/chrome depends on
the drm/drm_privacy_screen_driver.h header in your tree.

Thanks!

> ---
> v5: * Add blank line, 2 more vowels to the driver name as per comments
>     * Add Dmitry's "Reviewed by"
> v4: Same as v3 (No changes)
> v3: * Renamed everything chromeos_priv_scrn_* to chromeos_privacy_screen_*
>      (and added line breaks to accommodate longer names within 80 chars)
>     * Cleanup / Added few comments
>     * Use the newly added drm_privacy_screen_get_drvdata()
>     * Provide the cleanup function chromeos_privacy_screen_remove()
> v2: * Reword the commit log
>     * Make the Kconfig into a tristate
>     * Reorder the patches in the series.
>=20
>  drivers/platform/chrome/Kconfig               |  11 ++
>  drivers/platform/chrome/Makefile              |   1 +
>  .../platform/chrome/chromeos_privacy_screen.c | 153 ++++++++++++++++++
>  3 files changed, 165 insertions(+)
>  create mode 100644 drivers/platform/chrome/chromeos_privacy_screen.c
>=20
> diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kc=
onfig
> index ccc23d8686e8..75e93efd669f 100644
> --- a/drivers/platform/chrome/Kconfig
> +++ b/drivers/platform/chrome/Kconfig
> @@ -243,6 +243,17 @@ config CROS_USBPD_NOTIFY
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called cros_usbpd_notify.
> =20
> +config CHROMEOS_PRIVACY_SCREEN
> +	tristate "ChromeOS Privacy Screen support"
> +	depends on ACPI
> +	depends on DRM
> +	select DRM_PRIVACY_SCREEN
> +	help
> +	  This driver provides the support needed for the in-built electronic
> +	  privacy screen that is present on some ChromeOS devices. When enabled,
> +	  this should probably always be built into the kernel to avoid or
> +	  minimize drm probe deferral.
> +
>  source "drivers/platform/chrome/wilco_ec/Kconfig"
> =20
>  endif # CHROMEOS_PLATFORMS
> diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/M=
akefile
> index f901d2e43166..5d4be9735d9d 100644
> --- a/drivers/platform/chrome/Makefile
> +++ b/drivers/platform/chrome/Makefile
> @@ -4,6 +4,7 @@
>  CFLAGS_cros_ec_trace.o:=3D		-I$(src)
> =20
>  obj-$(CONFIG_CHROMEOS_LAPTOP)		+=3D chromeos_laptop.o
> +obj-$(CONFIG_CHROMEOS_PRIVACY_SCREEN)	+=3D chromeos_privacy_screen.o
>  obj-$(CONFIG_CHROMEOS_PSTORE)		+=3D chromeos_pstore.o
>  obj-$(CONFIG_CHROMEOS_TBMC)		+=3D chromeos_tbmc.o
>  obj-$(CONFIG_CROS_EC)			+=3D cros_ec.o
> diff --git a/drivers/platform/chrome/chromeos_privacy_screen.c b/drivers/=
platform/chrome/chromeos_privacy_screen.c
> new file mode 100644
> index 000000000000..77e9f5ee8e33
> --- /dev/null
> +++ b/drivers/platform/chrome/chromeos_privacy_screen.c
> @@ -0,0 +1,153 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + *  ChromeOS Privacy Screen support
> + *
> + * Copyright (C) 2022 Google LLC
> + *
> + * This is the Chromeos privacy screen provider, present on certain chro=
mebooks,
> + * represented by a GOOG0010 device in the ACPI. This ACPI device, if pr=
esent,
> + * will cause the i915 drm driver to probe defer until this driver regis=
ters
> + * the privacy-screen.
> + */
> +
> +#include <linux/acpi.h>
> +#include <drm/drm_privacy_screen_driver.h>
> +
> +/*
> + * The DSM (Device Specific Method) constants below are the agreed API w=
ith
> + * the firmware team, on how to control privacy screen using ACPI method=
s.
> + */
> +#define PRIV_SCRN_DSM_REVID		1	/* DSM version */
> +#define PRIV_SCRN_DSM_FN_GET_STATUS	1	/* Get privacy screen status */
> +#define PRIV_SCRN_DSM_FN_ENABLE		2	/* Enable privacy screen */
> +#define PRIV_SCRN_DSM_FN_DISABLE	3	/* Disable privacy screen */
> +
> +static const guid_t chromeos_privacy_screen_dsm_guid =3D
> +		    GUID_INIT(0xc7033113, 0x8720, 0x4ceb,
> +			      0x90, 0x90, 0x9d, 0x52, 0xb3, 0xe5, 0x2d, 0x73);
> +
> +static void
> +chromeos_privacy_screen_get_hw_state(struct drm_privacy_screen
> +				     *drm_privacy_screen)
> +{
> +	union acpi_object *obj;
> +	acpi_handle handle;
> +	struct device *privacy_screen =3D
> +		drm_privacy_screen_get_drvdata(drm_privacy_screen);
> +
> +	handle =3D acpi_device_handle(to_acpi_device(privacy_screen));
> +	obj =3D acpi_evaluate_dsm(handle, &chromeos_privacy_screen_dsm_guid,
> +				PRIV_SCRN_DSM_REVID,
> +				PRIV_SCRN_DSM_FN_GET_STATUS, NULL);
> +	if (!obj) {
> +		dev_err(privacy_screen,
> +			"_DSM failed to get privacy-screen state\n");
> +		return;
> +	}
> +
> +	if (obj->type !=3D ACPI_TYPE_INTEGER)
> +		dev_err(privacy_screen,
> +			"Bad _DSM to get privacy-screen state\n");
> +	else if (obj->integer.value =3D=3D 1)
> +		drm_privacy_screen->hw_state =3D drm_privacy_screen->sw_state =3D
> +			PRIVACY_SCREEN_ENABLED;
> +	else
> +		drm_privacy_screen->hw_state =3D drm_privacy_screen->sw_state =3D
> +			PRIVACY_SCREEN_DISABLED;
> +
> +	ACPI_FREE(obj);
> +}
> +
> +static int
> +chromeos_privacy_screen_set_sw_state(struct drm_privacy_screen
> +				     *drm_privacy_screen,
> +				     enum drm_privacy_screen_status state)
> +{
> +	union acpi_object *obj =3D NULL;
> +	acpi_handle handle;
> +	struct device *privacy_screen =3D
> +		drm_privacy_screen_get_drvdata(drm_privacy_screen);
> +
> +	handle =3D acpi_device_handle(to_acpi_device(privacy_screen));
> +
> +	if (state =3D=3D PRIVACY_SCREEN_DISABLED) {
> +		obj =3D acpi_evaluate_dsm(handle,
> +					&chromeos_privacy_screen_dsm_guid,
> +					PRIV_SCRN_DSM_REVID,
> +					PRIV_SCRN_DSM_FN_DISABLE, NULL);
> +	} else if (state =3D=3D PRIVACY_SCREEN_ENABLED) {
> +		obj =3D acpi_evaluate_dsm(handle,
> +					&chromeos_privacy_screen_dsm_guid,
> +					PRIV_SCRN_DSM_REVID,
> +					PRIV_SCRN_DSM_FN_ENABLE, NULL);
> +	} else {
> +		dev_err(privacy_screen,
> +			"Bad attempt to set privacy-screen status to %u\n",
> +			state);
> +		return -EINVAL;
> +	}
> +
> +	if (!obj) {
> +		dev_err(privacy_screen,
> +			"_DSM failed to set privacy-screen state\n");
> +		return -EIO;
> +	}
> +
> +	drm_privacy_screen->hw_state =3D drm_privacy_screen->sw_state =3D state;
> +	ACPI_FREE(obj);
> +	return 0;
> +}
> +
> +static const struct drm_privacy_screen_ops chromeos_privacy_screen_ops =
=3D {
> +	.get_hw_state =3D chromeos_privacy_screen_get_hw_state,
> +	.set_sw_state =3D chromeos_privacy_screen_set_sw_state,
> +};
> +
> +static int chromeos_privacy_screen_add(struct acpi_device *adev)
> +{
> +	struct drm_privacy_screen *drm_privacy_screen =3D
> +		drm_privacy_screen_register(&adev->dev,
> +					    &chromeos_privacy_screen_ops,
> +					    &adev->dev);
> +
> +	if (IS_ERR(drm_privacy_screen)) {
> +		dev_err(&adev->dev, "Error registering privacy-screen\n");
> +		return PTR_ERR(drm_privacy_screen);
> +	}
> +
> +	adev->driver_data =3D drm_privacy_screen;
> +	dev_info(&adev->dev, "registered privacy-screen '%s'\n",
> +		 dev_name(&drm_privacy_screen->dev));
> +
> +	return 0;
> +}
> +
> +static int chromeos_privacy_screen_remove(struct acpi_device *adev)
> +{
> +	struct drm_privacy_screen *drm_privacy_screen =3D	acpi_driver_data(adev=
);
> +
> +	drm_privacy_screen_unregister(drm_privacy_screen);
> +	return 0;
> +}
> +
> +static const struct acpi_device_id chromeos_privacy_screen_device_ids[] =
=3D {
> +	{"GOOG0010", 0}, /* Google's electronic privacy screen for eDP-1 */
> +	{}
> +};
> +MODULE_DEVICE_TABLE(acpi, chromeos_privacy_screen_device_ids);
> +
> +static struct acpi_driver chromeos_privacy_screen_driver =3D {
> +	.name =3D "chromeos_privacy_screen_driver",
> +	.class =3D "ChromeOS",
> +	.ids =3D chromeos_privacy_screen_device_ids,
> +	.ops =3D {
> +		.add =3D chromeos_privacy_screen_add,
> +		.remove =3D chromeos_privacy_screen_remove,
> +	},
> +};
> +
> +module_acpi_driver(chromeos_privacy_screen_driver);
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("ChromeOS ACPI Privacy Screen driver");
> +MODULE_AUTHOR("Rajat Jain <rajatja@google.com>");
> --=20
> 2.34.1.575.g55b058a8bb-goog
>=20

--=20
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

--Qn5xF3c+Hpu80EQa
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQQCtZK6p/AktxXfkOlzbaomhzOwwgUCYdiP8wAKCRBzbaomhzOw
wrPtAQDWY6LxjaP+hjlbFvWHyp62Z5zH/NXbjBZ16wQYdzBP5wEAmDgvohz5vdRt
1cX0V5wjBcL2+niXGiaaWrQsHVE18AM=
=0p4J
-----END PGP SIGNATURE-----

--Qn5xF3c+Hpu80EQa--