On Tue, May 17, 2016 at 04:30:41PM +0300, Ander Conselvan De Oliveira wrote:
> On Tue, 2016-05-17 at 14:26 +0200, Daniel Vetter wrote:
> > On Fri, May 13, 2016 at 05:14:57PM +0300, Ander Conselvan de Oliveira wrote:
> > >
> > > Hi,
> > >
> > > This series moves all of the calls to vlv_dpio_{read,write} to
> > > intel_dpio_phy.c. I think it simplifies the surrounding code a bit.
> > You still owe us all the kerneldoc for the intel_dpll_mgr.c extraction.
>
> https://patchwork.freedesktop.org/series/7294/
>
> > I think better to complete one extraction before starting the next one,
> > resulting in an even bigger mess than what we had before.
>
> This is actually part of the same thing. These are prep patches for moving
> VLV/CHV into the dpll infrastructure. But fair enough.
>
>
> But I have to disagree this would create an even bigger mess. There is so much
> code in intel_display.c that most static functions there are the equivalent of
> an undocumented non-static function elsewhere. And since they are in the same
> pile of 400+ functions, it is not obvious the documentation is missing. So I'd
> claim splitting code out of intel_display.c, even if without documentation, is
> an improvement.
The problem is that we've done this for some of the atomic work and
fumbled the job, so now there's also a bunch of non-static functions that
should be static but can't because they ended up split across .c files. It
is possible to make it worse ;-)
> With the current rules we transfer the burden of writing documentation from the
> person that made intel_display.c longer to the one trying to make it smaller.
> Maybe we should have an exception that everything in intel_display.c needs
> kerneldoc?
See above, I think writing docs is a crucial step of making things
actually more orthogonal, instead of just smearing it across more source
files. And from my quick review of the dpll doc patch I think we
can/should do better - at least if you don't look at kerneldoc as just a
typing exercise, but as an opportunity to really review everything.
And yes there's a problem with shifting the work, but I think the correct
fix for that is by volunteering the offenders who make intel_display.c
bigger to help out with cleaning up. Not by making it easier on those that
clean up, since that doesn't fix the source of the problem.
And I tried to help out in the past with a few ideas around extracting the
crtc platform support code, but those all died in bikesheds :(
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
