On Wed, Feb 01, 2012 at 02:05:38PM +0100, Sascha Hauer wrote:
> On Wed, Feb 01, 2012 at 07:55:40AM -0500, David Airlie wrote:
> >
> >
> > > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > > > index 8d593ad..cdbbb40 100644
> > > > --- a/include/drm/drm_crtc.h
> > > > +++ b/include/drm/drm_crtc.h
> > > > @@ -394,7 +394,7 @@ struct drm_crtc {
> > > > s64 framedur_ns, linedur_ns, pixeldur_ns;
> > > >
> > > > /* if you are using the helper */
> > > > - void *helper_private;
> > > > + struct drm_crtc_helper_funcs *helper_private;
> > > > };
> > > >
> > > >
> > > > @@ -481,7 +481,7 @@ struct drm_encoder {
> > > >
> > > > struct drm_crtc *crtc;
> > > > const struct drm_encoder_funcs *funcs;
> > > > - void *helper_private;
> > > > + struct drm_encoder_helper_funcs *helper_private;
> > > > };
> > > >
> > > > enum drm_connector_force {
> > > > @@ -573,7 +573,7 @@ struct drm_connector {
> > > > /* requested DPMS state */
> > > > int dpms;
> > > >
> > > > - void *helper_private;
> > > > + struct drm_connector_helper_funcs *helper_private;
> > > >
> > > > /* forced on connector */
> > > > enum drm_connector_force force;
> > >
> > > This is a separate chunk.
> >
> > And totally wrong, using the helper should remain optional, and the
> > helper includes should not be included into the main headers.
>
> The intention was to prevent people from thinking that they
> should invent their own set of helper functions. As these
> are only pointers we could also add a
>
> struct drm_crtc_helper_funcs;
> struct drm_connector_helper_funcs;
> struct drm_encoder_helper_funcs;
>
> to drm_crtc.h without having to include the helper includes.
I like this, it makes it a notch more obvious what these are for. And we
have precedence for slightly violating abstraction in this way: E.g. the
vfs isn't splattered with void pointers just because you're free to ignore
a lot of stuff (like the pagecache) when implementing an fs ...
Cheers, Daniel
--
Daniel Vetter
Mail: daniel at ffwll.ch
Mobile: +41 (0)79 365 57 48
