Re: [PATCH] drm/915: Parsing the missed out DTD fields from the VBT

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Dec 22, 2016 at 01:07:44PM -0500, Madhav Chauhan 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.
> 
> V2: Renamed lvds dvo timing structure members and code identation
> fix (Jani's review comments)
> 
> Signed-off-by: Vincente Tsou <vincente.tsou@intel.com>
> Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_bios.c     |  8 +++++---
>  drivers/gpu/drm/i915/intel_vbt_defs.h | 12 +++++++-----
>  2 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index a359def..f6d3755 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_lo);
>  	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_lo);
>  	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_lo);
>  	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..a92e776 100644
> --- a/drivers/gpu/drm/i915/intel_vbt_defs.h
> +++ b/drivers/gpu/drm/i915/intel_vbt_defs.h
> @@ -399,10 +399,12 @@ struct lvds_dvo_timing {
>  	u8 vblank_hi:4;
>  	u8 vactive_hi:4;
>  	u8 hsync_off_lo;
> -	u8 hsync_pulse_width;
> -	u8 vsync_pulse_width:4;
> -	u8 vsync_off:4;
> -	u8 rsvd0:6;
> +	u8 hsync_pulse_width_lo;
> +	u8 vsync_pulse_width_lo:4;
> +	u8 vsync_off_lo:4;
> +	u8 vsync_pulse_width_hi:2;
> +	u8 vsync_off_hi:2;
> +	u8 hsync_pulse_width_hi:2;

Last time I looked at this I decided that we had at least two
definitions for this thing in our code. But maybe that was just on the
igt side because I can't find it in the kernel right now.

Oh, and in igt someone has also realized that this stuff matches the
EDID DTD as well, and so we sometimes parse it using some EDIDy macros
instead. So three ways to do it in igt, at least one in the kernel (and
a second one if you count the EDID parser).  It might make sense to
clean it all up somehow.

>  	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 non_interlaced:1;
>  } __packed;
>  
>  struct lvds_pnp_id {
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx