| Message ID | 1481792500-30863-10-git-send-email-madhav.chauhan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 15 Dec 2016, Madhav Chauhan <madhav.chauhan@intel.com> wrote: > From: Vincente Tsou <vincente.tsou@intel.com> > > 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 <madhav.chauhan@intel.com> > Signed-off-by: Vincente Tsou <vincente.tsou@intel.com> 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. BR, Jani. > } __packed; > > struct lvds_pnp_id {
> -----Original Message----- > From: Nikula, Jani > Sent: Thursday, December 22, 2016 5:09 PM > To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: Conselvan De Oliveira, Ander <ander.conselvan.de.oliveira@intel.com>; > Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra > <chandra.konduru@intel.com>; Shankar, Uma <uma.shankar@intel.com>; > Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit > <shobhit.kumar@intel.com>; Tsou, Vincente <vincente.tsou@intel.com>; > Chauhan, Madhav <madhav.chauhan@intel.com> > 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 <madhav.chauhan@intel.com> > wrote: > > From: Vincente Tsou <vincente.tsou@intel.com> > > > > 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 <madhav.chauhan@intel.com> > > Signed-off-by: Vincente Tsou <vincente.tsou@intel.com> > > 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
On Thu, 22 Dec 2016, "Chauhan, Madhav" <madhav.chauhan@intel.com> wrote: >> -----Original Message----- >> From: Nikula, Jani >> Sent: Thursday, December 22, 2016 5:09 PM >> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel- >> gfx@lists.freedesktop.org >> Cc: Conselvan De Oliveira, Ander <ander.conselvan.de.oliveira@intel.com>; >> Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra >> <chandra.konduru@intel.com>; Shankar, Uma <uma.shankar@intel.com>; >> Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit >> <shobhit.kumar@intel.com>; Tsou, Vincente <vincente.tsou@intel.com>; >> Chauhan, Madhav <madhav.chauhan@intel.com> >> 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 <madhav.chauhan@intel.com> >> wrote: >> > From: Vincente Tsou <vincente.tsou@intel.com> >> > >> > 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 <madhav.chauhan@intel.com> >> > Signed-off-by: Vincente Tsou <vincente.tsou@intel.com> >> >> 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. Please send it as a separate patch outside of this series, it's not really related. BR, Jani. > >> >> BR, >> Jani. >> >> > } __packed; >> > >> > struct lvds_pnp_id { >> >> -- >> Jani Nikula, Intel Open Source Technology Center
> -----Original Message----- > From: Nikula, Jani > Sent: Thursday, December 22, 2016 7:13 PM > To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: Conselvan De Oliveira, Ander <ander.conselvan.de.oliveira@intel.com>; > Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra > <chandra.konduru@intel.com>; Shankar, Uma <uma.shankar@intel.com>; > Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit > <shobhit.kumar@intel.com>; Tsou, Vincente <vincente.tsou@intel.com> > Subject: RE: [GLK MIPI DSI V2 9/9] drm/915: Parsing the missed out DTD > fields from the VBT > > On Thu, 22 Dec 2016, "Chauhan, Madhav" <madhav.chauhan@intel.com> > wrote: > >> -----Original Message----- > >> From: Nikula, Jani > >> Sent: Thursday, December 22, 2016 5:09 PM > >> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel- > >> gfx@lists.freedesktop.org > >> Cc: Conselvan De Oliveira, Ander > >> <ander.conselvan.de.oliveira@intel.com>; > >> Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra > >> <chandra.konduru@intel.com>; Shankar, Uma > <uma.shankar@intel.com>; > >> Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit > >> <shobhit.kumar@intel.com>; Tsou, Vincente <vincente.tsou@intel.com>; > >> Chauhan, Madhav <madhav.chauhan@intel.com> > >> 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 <madhav.chauhan@intel.com> > >> wrote: > >> > From: Vincente Tsou <vincente.tsou@intel.com> > >> > > >> > 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 <madhav.chauhan@intel.com> > >> > Signed-off-by: Vincente Tsou <vincente.tsou@intel.com> > >> > >> 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. > > Please send it as a separate patch outside of this series, it's not really related. Agree, had same thought when V1 series was sent. Included it so that review and merge can be combined :) > > BR, > Jani. > > > > > >> > >> BR, > >> Jani. > >> > >> > } __packed; > >> > > >> > struct lvds_pnp_id { > >> > >> -- > >> Jani Nikula, Intel Open Source Technology Center > > -- > Jani Nikula, Intel Open Source Technology Center
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); 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; 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; } __packed; struct lvds_pnp_id {