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

Jani Nikula
2016-12-23T07:03:37Z

On Thu, 22 Dec 2016, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> 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.

Agreed, but this is a worthwhile addition to the kernel right now.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

>
>> 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

--
Jani Nikula, Intel Open Source Technology Center