Re: [PATCH 0/6] Move dpio access out of intel_display.c

[Ander Conselvan De Oliveira <conselvan2@gmail.com>]

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.

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?

Ander


> -Daniel
> 
> > 
> > 
> > Thanks,
> > Ander
> > 
> > Ander Conselvan de Oliveira (6):
> >   drm/i915: Rename struct dpll to struct intel_dpll
> >   drm/i915: Move dpio code of VLV/CHV dpll enabling to intel_dpio_phy.c
> >   drm/i915: Merge vlv/chv _prepare_pll() with their enable counterpart
> >   drm/i915: Move VLV divider readout to intel_dpio_phy.c
> >   drm/i915: Move CHV divider readout to intel_dpio_phy.c
> >   drm/i915: Move toggling of CHV DPIO_DCLKP_EN to intel_dpio_phy.c
> > 
> >  drivers/gpu/drm/i915/i915_drv.h       |  14 +-
> >  drivers/gpu/drm/i915/intel_ddi.c      |   2 +-
> >  drivers/gpu/drm/i915/intel_display.c  | 395 ++++++-------------------------
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c       |   2 +-
> >  drivers/gpu/drm/i915/intel_dpio_phy.c | 260 ++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_dpll_mgr.c |   2 +-
> >  drivers/gpu/drm/i915/intel_drv.h      |  10 +-
> >  drivers/gpu/drm/i915/intel_sdvo.c     |   2 +-
> >  8 files changed, 350 insertions(+), 337 deletions(-)
> > 
> > -- 
> > 2.5.5
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx