lkml.org 
[lkml]   [2012]   [Aug]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [Intel-gfx] [PATCH 2/7] drm/i915: FDI B/C share 4 lanes on Ivybridge
Date
"Lespiau, Damien" <damien.lespiau@intel.com> writes:

> On Tue, Aug 14, 2012 at 5:34 AM, Keith Packard <keithp@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@intel.com
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2012-08-17 17:41    [W:0.412 / U:0.124 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site