"Lespiau, Damien" <damien.lespiau at intel.com> writes: > On Tue, Aug 14, 2012 at 5:34 AM, Keith Packard <keithp at keithp.com> wrote: > > @@ -3728,7 +3728,8 @@ static inline bool intel_panel_use_ssc(struct > drm_i915_private *dev_priv) > */ > static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc, > unsigned int *pipe_bpp, > - struct drm_display_mode *mode) > + struct drm_display_mode *mode, > + int max_fdi_bpp) > > There's some kernel-doc for this function, maybe add a @max_fdi_bpp > there? Will do > This chunk is being moved around in a later patch in the series, > merging the two patches in one looks like a good idea? Or at least move this into its final position in this patch. > I guess this does not cover the case of pipe B using 3 lanes meaning > pipe C can use 1? It didn't look like that was a supported mode from the docs. > This duplicates the code just that is just a few lines away, instead > how about moving the logic to set target_clock up in front of this > whole if()? It's not the same, it's the inverse -- computing bpp from lanes+clock clock instead of computing lanes from bpp+clock. But, yeah, it would be nice to have these merged somehow. I couldn't figure out a good way though. > This chunk is also reworked in a later commit in this series. I'll see if I can't avoid that as it's confusing. Thanks for your review! -- keith.packard at intel.com -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 827 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20120817/d1952604/attachment.pgp>