From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH v2] drm: Add Grain Media GM12U320 kms driver Date: Fri, 3 Jun 2016 19:27:53 +0200 Message-ID: <20160603172753.GD21013@ulmo.ba.sec> References: <1464959192-21064-1-git-send-email-hdegoede@redhat.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1580159812==" Return-path: Received: from mail-wm0-x241.google.com (mail-wm0-x241.google.com [IPv6:2a00:1450:400c:c09::241]) by gabe.freedesktop.org (Postfix) with ESMTPS id A681D6EEAE for ; Fri, 3 Jun 2016 17:27:57 +0000 (UTC) Received: by mail-wm0-x241.google.com with SMTP id e3so894210wme.2 for ; Fri, 03 Jun 2016 10:27:57 -0700 (PDT) In-Reply-To: <1464959192-21064-1-git-send-email-hdegoede@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Hans de Goede Cc: Dave Airlie , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============1580159812== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Hf61M2y+wYpnELGG" Content-Disposition: inline --Hf61M2y+wYpnELGG Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jun 03, 2016 at 03:06:32PM +0200, Hans de Goede wrote: > Grain-media GM12U320 based devices are mini video projectors using USB for > both power and video data transport. >=20 > This commit adds a kms driver for these devices, including prime support. >=20 > This driver is based on the existing udl kms driver, and the gm12u320 > fb driver by Viacheslav Nurmekhamitov . >=20 > Signed-off-by: Hans de Goede > --- > Changes in v2: > -Rebase on 4.7-rc1 > -Sync with udl, bring in any fixes done to udl since v1 This sounds a little nightmarish in the long term. From a cursory look this duplicates a bunch of code that's present in UDL, where the main differences are EDID handling and data transfer. I wonder if we can't find a better way of reusing code than duplicating it. > diff --git a/drivers/gpu/drm/gm12u320/gm12u320_connector.c b/drivers/gpu/= drm/gm12u320/gm12u320_connector.c [...] > +/* > + * Note this assumes this driver is only ever used with the Acer C120, i= f we > + * add support for other devices the vendor and model should be paramete= rized. > + */ > +static struct edid gm12u320_edid =3D { > + .header =3D { 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00 }, > + .mfg_id =3D { 0x04, 0x72 }, /* "ACR" */ > + .prod_code =3D { 0x20, 0xc1 }, /* C120h */ > + .mfg_week =3D 1, > + .mfg_year =3D 1, > + .version =3D 1, /* EDID 1.3 */ > + .revision =3D 3, /* EDID 1.3 */ > + .input =3D 0x80, /* Digital input */ > + .features =3D 0x02, /* Pref timing in DTD 1 */ > + .standard_timings =3D { { 1, 1 }, { 1, 1 }, { 1, 1 }, { 1, 1 }, > + { 1, 1 }, { 1, 1 }, { 1, 1 }, { 1, 1 } }, > + .detailed_timings =3D { { > + .pixel_clock =3D 3383, > + /* hactive =3D 852, hblank =3D 256 */ > + .data.pixel_data.hactive_lo =3D 0x54, > + .data.pixel_data.hblank_lo =3D 0x00, > + .data.pixel_data.hactive_hblank_hi =3D 0x31, > + /* vactive =3D 480, vblank =3D 28 */ > + .data.pixel_data.vactive_lo =3D 0xe0, > + .data.pixel_data.vblank_lo =3D 0x1c, > + .data.pixel_data.vactive_vblank_hi =3D 0x10, > + /* hsync offset 40 pw 128, vsync offset 1 pw 4 */ > + .data.pixel_data.hsync_offset_lo =3D 0x28, > + .data.pixel_data.hsync_pulse_width_lo =3D 0x80, > + .data.pixel_data.vsync_offset_pulse_width_lo =3D 0x14, > + .data.pixel_data.hsync_vsync_offset_pulse_width_hi =3D 0x00, > + /* Digital separate syncs, hsync+, vsync+ */ > + .data.pixel_data.misc =3D 0x1e, > + }, { > + .pixel_clock =3D 0, > + .data.other_data.type =3D 0xfd, /* Monitor ranges */ > + .data.other_data.data.range.min_vfreq =3D 59, > + .data.other_data.data.range.max_vfreq =3D 61, > + .data.other_data.data.range.min_hfreq_khz =3D 29, > + .data.other_data.data.range.max_hfreq_khz =3D 32, > + .data.other_data.data.range.pixel_clock_mhz =3D 4, /* 40 MHz */ > + .data.other_data.data.range.flags =3D 0, > + .data.other_data.data.range.formula.cvt =3D { > + 0xa0, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20 }, > + }, { > + .pixel_clock =3D 0, > + .data.other_data.type =3D 0xfc, /* Model string */ > + .data.other_data.data.str.str =3D { > + 'C', '1', '2', '0', 'P', 'r', 'o', 'j', 'e', 'c', > + 't', 'o', 'r' }, > + }, { > + .pixel_clock =3D 0, > + .data.other_data.type =3D 0xfe, /* Unspecified text / padding */ > + .data.other_data.data.str.str =3D { > + '\n', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', > + ' ', ' ', ' ' }, > + } }, > + .checksum =3D 0x40, > +}; Where did you get this from? Doesn't this device have a chip that you can query at runtime? > +static int gm12u320_connector_set_property(struct drm_connector *connect= or, > + struct drm_property *property, > + uint64_t val) > +{ > + return 0; > +} Just drop it if it doesn't do anything. > +int gm12u320_connector_init(struct drm_device *dev, > + struct drm_encoder *encoder) > +{ > + struct drm_connector *connector; > + > + connector =3D kzalloc(sizeof(struct drm_connector), GFP_KERNEL); > + if (!connector) > + return -ENOMEM; > + > + drm_connector_init(dev, connector, &gm12u320_connector_funcs, > + DRM_MODE_CONNECTOR_Unknown); According to the Grain Media website this chip is a USB 3.0-to-HDMI bridge, so DRM_MODE_CONNECTOR_HDMIA might be a better choice here. > diff --git a/drivers/gpu/drm/gm12u320/gm12u320_drv.c b/drivers/gpu/drm/gm= 12u320/gm12u320_drv.c [...] > new file mode 100644 > index 0000000..eff3a44 > --- /dev/null > +++ b/drivers/gpu/drm/gm12u320/gm12u320_drv.c > @@ -0,0 +1,160 @@ > +/* > + * Copyright (C) 2012-2016 Red Hat Inc. > + * > + * This file is subject to the terms and conditions of the GNU General P= ublic > + * License v2. See the file COPYING in the main directory of this archiv= e for > + * more details. > + */ > + > +#include > +#include > +#include > +#include "gm12u320_drv.h" > + > +static int gm12u320_driver_set_busid(struct drm_device *d, struct drm_ma= ster *m) > +{ > + return 0; > +} This is optional, you can drop it. Thierry --Hf61M2y+wYpnELGG Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJXUb4XAAoJEN0jrNd/PrOh74sP/jh0y48i9H9FNtcQJ/CbKIKy uyX66Wbxhrek7ypDuqACIsHmrxmsnnuLWOKIBtQZmQaqDuxViXX3Ps/7bPD6WJNz tgS/hz2AdXM03ZELxly0WfFnlCXC96kCqG3KZnuqdQB4oWtPNSErB7Hpg05tus21 GjsX6Xldi0pLOuf+2J95HcStVnwHL5kINml5RpnKpzoni/EX2OP4ALJP8DMZ6xgu 0gb8hxMS5bpVPt9k4zIfuY7AoCcNE+7lFHacvIf8uIdwi5VlK9UFaY6SOvhI2ytj DfZi2DnztlexF5Z9AASoEXUG4EjYnAlFvRw8rr6Y9SzWEg4URWXaPSDmWvTN0DqK npXpIs239HIRb5XFMAsMoOO37FBP6479EmeKlBKKiiJxMUYk2I5g8sPhchSR756e V6zhm5phkPbuVxkxGF8vLKErpWW/bLXSiECVe+EmGc5zm47Q7WnYtSbpGctADtfF LNDA9ZztUHlCg1DjYB4AxGNmXv6ck4HxDhFKRAwpgVNxfv+Zw8Hp0BPM3Z6XQ69B z52OaA2SmSod6nEM9HjLajahig5erpJsnh5BycThvbAlFkN7PaBkSNU7X/gKhJyV kiHypqVLHd2lNRJMdxEN0sjpvfaz/WIXj02ylrINro0CpT7l9/wOH1VvHZVCzhob oZSDFS1Krm0gRxMJI0ME =hXoL -----END PGP SIGNATURE----- --Hf61M2y+wYpnELGG-- --===============1580159812== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============1580159812==--