From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH 01/28] drm: Polish fbdev helper struct docs Date: Mon, 7 Dec 2015 12:53:48 +0100 Message-ID: <20151207115348.GQ13177@ulmo> References: <1449218769-16577-1-git-send-email-daniel.vetter@ffwll.ch> <1449218769-16577-2-git-send-email-daniel.vetter@ffwll.ch> <20151207104522.GD13177@ulmo> <20151207115024.GD10243@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1810891717==" Return-path: In-Reply-To: <20151207115024.GD10243@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter Cc: Daniel Vetter , Intel Graphics Development , DRI Development , Daniel Vetter List-Id: intel-gfx@lists.freedesktop.org --===============1810891717== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Evp0T6PXIQ26qRMT" Content-Disposition: inline --Evp0T6PXIQ26qRMT Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Dec 07, 2015 at 12:50:24PM +0100, Daniel Vetter wrote: > On Mon, Dec 07, 2015 at 11:45:22AM +0100, Thierry Reding wrote: > > On Fri, Dec 04, 2015 at 09:45:42AM +0100, Daniel Vetter wrote: > > > Mostly this is just adding extensive docs for the callbacks, but also > > > a few other additions. > > >=20 > > > v2: Use FIXME comments to annotate helper hooks that should be > > > replaced. > > >=20 > > > Signed-off-by: Daniel Vetter > > > --- > > > include/drm/drm_fb_helper.h | 92 +++++++++++++++++++++++++++++++++++= +++------- > > > 1 file changed, 79 insertions(+), 13 deletions(-) > > >=20 > > > diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h > > > index 87b090c4b730..5ce033e36039 100644 > > > --- a/include/drm/drm_fb_helper.h > > > +++ b/include/drm/drm_fb_helper.h > > > @@ -74,25 +74,76 @@ struct drm_fb_helper_surface_size { > > > =20 > > > /** > > > * struct drm_fb_helper_funcs - driver callbacks for the fbdev emula= tion library > > > - * @gamma_set: Set the given gamma lut register on the given crtc. > > > - * @gamma_get: Read the given gamma lut register on the given crtc, = used to > > > - * save the current lut when force-restoring the fbdev f= or e.g. > > > - * kdbg. > > > - * @fb_probe: Driver callback to allocate and initialize the fbdev i= nfo > > > - * structure. Furthermore it also needs to allocate the d= rm > > > - * framebuffer used to back the fbdev. > > > - * @initial_config: Setup an initial fbdev display configuration > > > * > > > * Driver callbacks used by the fbdev emulation helper library. > > > */ > > > struct drm_fb_helper_funcs { > > > + /** > > > + * @gamma_set: > > > + * > > > + * Set the given gamma lut register on the given crtc. > > > + * > > > + * This callback is optional. > > > + * > > > + * FIXME: > > > + * > > > + * This callback is functionally redundant with the core gamma table > > > + * support and simply exists because the fbdev hasn't yet been > > > + * refactored to use the core gamma table interfaces. > > > + */ > > > void (*gamma_set)(struct drm_crtc *crtc, u16 red, u16 green, > > > u16 blue, int regno); > >=20 > > Pardon my ignorance, but is there a way to document parameters with this > > new syntax? >=20 > Unfortunately not. Doing that would be quite a bit more rework of the > entire kerneldoc toolchain I think. Yes, that was my suspicion as well. > > > + /** > > > + * @gamma_get: > > > + * > > > + * Read the given gamma lut register on the given crtc, used to sav= e the > > > + * current lut when force-restoring the fbdev for e.g. kdbg. > > > + * > > > + * This callback is optional. > > > + * > > > + * FIXME: > > > + * > > > + * This callback is functionally redundant with the core gamma table > > > + * support and simply exists because the fbdev hasn't yet been > > > + * refactored to use the core gamma table interfaces. > > > + */ > > > void (*gamma_get)(struct drm_crtc *crtc, u16 *red, u16 *green, > > > u16 *blue, int regno); > >=20 > > Nit: While at it, perhaps (here and in the @gamma_set documentation): > > s/lut/LUT/ and s/crtc/CRTC/? >=20 > Yeah I thought about our inconsistency wrt upper-case of abbrevations in > the docs. I think we should do this as a trivial patch thing for newbies. Fair enough. Thierry > > > * @fb: Scanout framebuffer object > > > * @dev: DRM device > >=20 > > There seems to be an extra space between the : and the description. That > > was already there, but maybe worth a follow-up. >=20 > I think fix that up while applying, same for the others. Okay, either way, this is a good improvement, so: Reviewed-by: Thierry Reding --Evp0T6PXIQ26qRMT Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJWZXNMAAoJEN0jrNd/PrOh/XIP/3h+l0m9Wcbo+Mpv7yJRnfDF LPTI26aSTauLu+EfdIEr1O2hezLbOxrBmp3FQ8Tr0c33p2DDY6ik9+kVsiRoi6fw h1BcJsaTOlq0bc/q9kNjAqzQYEtOOC0ZKIAijlKX1KbgQTV7Y+4ZkYkQpj6lbz1b Uv/VaKMJC+hilwofsYXDWDaCV0UJ5TGh48zkj+ZYooHGXv8owJ6oTDDqPxU9h0jv XTOAgnFTQ3WMiQQvqxClyQ/Yr0zDkwIlpaa7PvqA6LpI5FZoM1yjTJ23wE1ee6et iQZB0ewbIrPlP1/i7RffR4JXwovQIjdtMkdfwWVgvmArE7rQD/Ou4vQrPweB8KgC V5xswwlspUOYR4wXDBLjnwBQHfUxYdi4ocBkjYU26BHOL5vEiKJK2V1o5U8sYaBh vnKdrtLH1/H981zuIN60fn0abRoPra1I+HPBOqFdEXofGKmOr0TmABd3SRQyUJsm HFK4ZuWmOCEgSOwGZ1ZYgMi5WPdF8wRxMZDgrpekskjiKkgWqKmumggQ0UxDj30k 7CZ73tjqKx37NYRfo6Q/2+hzCV2vuwHJfoW2BrDZ/dNmXjVS4iWtpyYNa7SN7dn3 MXdULKu76+j/WiwduL4+NjggiryuMcfDTC3muFfp/L9RZVFKhgiN1+ZBGhMrq7Ox VF1QOhGw6LcssflFrhVp =uCFP -----END PGP SIGNATURE----- --Evp0T6PXIQ26qRMT-- --===============1810891717== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK --===============1810891717==--