> -----Original Message-----
> From: Nikula, Jani
> Sent: Thursday, December 22, 2016 5:09 PM
> To: Chauhan, Madhav <[email protected]>; intel-
> [email protected]
> Cc: Conselvan De Oliveira, Ander <[email protected]>;
> Saarinen, Jani <[email protected]>; Konduru, Chandra
> <[email protected]>; Shankar, Uma <[email protected]>;
> Mukherjee, Indranil <[email protected]>; Kumar, Shobhit
> <[email protected]>; Tsou, Vincente <[email protected]>;
> Chauhan, Madhav <[email protected]>
> Subject: Re: [GLK MIPI DSI V2 9/9] drm/915: Parsing the missed out DTD
> fields from the VBT
>
> On Thu, 15 Dec 2016, Madhav Chauhan <[email protected]>
> wrote:
> > From: Vincente Tsou <[email protected]>
> >
> > The upper bits of the vsync width, vsync offset and hsync width were
> > not parsed form the VBT. Parse these fields in this patch.
> >
> > Signed-off-by: Madhav Chauhan <[email protected]>
> > Signed-off-by: Vincente Tsou <[email protected]>
>
> The author Signed-off-by should be first, others are added below.
>
> > ---
> > drivers/gpu/drm/i915/intel_bios.c | 8 +++++---
> > drivers/gpu/drm/i915/intel_vbt_defs.h | 6 ++++--
> > 2 files changed, 9 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_bios.c
> > b/drivers/gpu/drm/i915/intel_bios.c
> > index eaade27..e1d014b 100644
> > --- a/drivers/gpu/drm/i915/intel_bios.c
> > +++ b/drivers/gpu/drm/i915/intel_bios.c
> > @@ -114,16 +114,18 @@ static u32 get_blocksize(const void *block_data)
> > panel_fixed_mode->hsync_start = panel_fixed_mode->hdisplay +
> > ((dvo_timing->hsync_off_hi << 8) | dvo_timing-
> >hsync_off_lo);
> > panel_fixed_mode->hsync_end = panel_fixed_mode->hsync_start +
> > - dvo_timing->hsync_pulse_width;
> > + ((dvo_timing->hsync_pulse_width_hi << 8) |
> > + dvo_timing->hsync_pulse_width);
> > panel_fixed_mode->htotal = panel_fixed_mode->hdisplay +
> > ((dvo_timing->hblank_hi << 8) | dvo_timing->hblank_lo);
> >
> > panel_fixed_mode->vdisplay = (dvo_timing->vactive_hi << 8) |
> > dvo_timing->vactive_lo;
> > panel_fixed_mode->vsync_start = panel_fixed_mode->vdisplay +
> > - dvo_timing->vsync_off;
> > + ((dvo_timing->vsync_off_hi << 4) | dvo_timing->vsync_off);
> > panel_fixed_mode->vsync_end = panel_fixed_mode->vsync_start +
> > - dvo_timing->vsync_pulse_width;
> > + ((dvo_timing->vsync_pulse_width_hi << 4) |
> > + dvo_timing->vsync_pulse_width);
>
> The indentation for the above changes seem to be off.
>
> > panel_fixed_mode->vtotal = panel_fixed_mode->vdisplay +
> > ((dvo_timing->vblank_hi << 8) | dvo_timing->vblank_lo);
> > panel_fixed_mode->clock = dvo_timing->clock * 10; diff --git
> > a/drivers/gpu/drm/i915/intel_vbt_defs.h
> > b/drivers/gpu/drm/i915/intel_vbt_defs.h
> > index 8886cab1..bf9d2d3 100644
> > --- a/drivers/gpu/drm/i915/intel_vbt_defs.h
> > +++ b/drivers/gpu/drm/i915/intel_vbt_defs.h
> > @@ -402,7 +402,9 @@ struct lvds_dvo_timing {
> > u8 hsync_pulse_width;
> > u8 vsync_pulse_width:4;
> > u8 vsync_off:4;
> > - u8 rsvd0:6;
> > + u8 vsync_pulse_width_hi:2;
> > + u8 vsync_off_hi:2;
> > + u8 hsync_pulse_width_hi:2;
>
> Please rename the lo counterparts of these fields to have _lo suffix, for
> consistency with other hi/lo split fields. With that, the compiler will help
> you
> in making sure you found all the places you need to fix. ;)
>
> > u8 hsync_off_hi:2;
> > u8 himage_lo;
> > u8 vimage_lo;
> > @@ -414,7 +416,7 @@ struct lvds_dvo_timing {
> > u8 digital:2;
> > u8 vsync_positive:1;
> > u8 hsync_positive:1;
> > - u8 rsvd2:1;
> > + u8 interlaced:1;
>
> This should be non_interlaced, as that's how the bit is defined.
>
> Otherwise, seems like a good find.
Thanks Jani for review. Will address the review comment and resend it.
>
> BR,
> Jani.
>
> > } __packed;
> >
> > struct lvds_pnp_id {
>
> --
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
