Re: [PATCH] drm/i915: don't chnage the original mode in dp_mode_fixup

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, May 30, 2012 at 11:58:43AM +0100, Chris Wilson wrote:
> On Wed, 30 May 2012 12:28:04 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > We should only frob adjusted_mode. This is in preparation of
> > a massive patch by Laurent Pinchart to make the mode argument
> > const.
> > 
> > The only thing we actually touch is mode->clock, but only if
> > it's a panel. And in that case we also set adjusted_mode->clock
> > to the same value. All the generic code already uses the
> > adjusted_mode exclusively, so we only have to move the dp
> > link bw calculations over to that. This requires a small
> > changes so that the shared code with mode_valid doesn't
> > touch the mode argument.
> 
> Separate patch please, I'm sure you are right, but that is the scary
> one...

Will do.

> > Also mark the mode argument of pch_panel_fitting const.
> > 
> > Reported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c    |   19 +++++++------------
> >  drivers/gpu/drm/i915/intel_drv.h   |    2 +-
> >  drivers/gpu/drm/i915/intel_panel.c |    2 +-
> >  3 files changed, 9 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 296cfc2..05c4748 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -222,7 +222,7 @@ intel_dp_max_data_rate(int max_link_clock, int max_lanes)
> >  static bool
> >  intel_dp_adjust_dithering(struct intel_dp *intel_dp,
> >  			  struct drm_display_mode *mode,
> > -			  struct drm_display_mode *adjusted_mode)
> > +			  bool adjust_mode)
> 
> Would this look more pleasant if you rewrote this function so that the
> adjustment of flags was done in the caller?

Well, I've added the adjusted_mode parameter originally to exactly avoid
this duplication of code between mode_fixup and mode_valid (which caused a
bug). Atm we could just pass back a needs_6bpc_dither flag, but I guess
we'll eventually need similar logic for higher bpc (and maybe other funky
stuff). So I think setting the flag here is ok and the least ugly version.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48