On Fri, 2011-12-09 at 11:46 +0000, Kavuri, Sateesh wrote:
> + if ((multi_val == STRUCTURE_PRESENT) ||
> + (multi_val == STRUCTURE_MASK_PRESENT) ) {
> + if ((edid_ext[i+15+hdmi_vic_len] & 0x01) ==
> 0x01)
> + caps->format |= 0x0; /* FRAME_PACKING */
> + if ((edid_ext[i+15+hdmi_vic_len] & 0x02) ==
> 0x02)
> + caps->format |= 0x1;
> /*FIELD_ALTERNATIVE */
> + if ((edid_ext[i+15+hdmi_vic_len] & 0x04) ==
> 0x04)
> + caps->format |= 0x2; /*
> LINE_ALTERNATIVE */
> + if ((edid_ext[i+15+hdmi_vic_len] & 0x08) ==
> 0x08)
> + caps->format |= 0x3;
> /*SIDE_BY_SIDE_FULL */
> + if ((edid_ext[i+15+hdmi_vic_len] & 0x10) ==
> 0x10)
> + caps->format |= 0x4; /* L_DEPTH */
> + if ((edid_ext[i+15+hdmi_vic_len] & 0x20) ==
> 0x20)
> + caps->format |= 0x5; /*
> L_DEPTH_GFX_GFX_DEPTH */
> + if ((edid_ext[i+15+hdmi_vic_len] & 0x40) ==
> 0x40)
> + caps->format |= 0x6; /* TOP_BOTTOM */
> + if ((edid_ext[i+14+hdmi_vic_len] & 0x01) ==
> 0x01)
> + caps->format |= 0x7; /* S_BY_S_HALF */
> + if ((edid_ext[i+14+hdmi_vic_len] & 0x80) ==
> 0x80)
> + caps->format |= 0x8; /*
> S_BY_S_HALF_QUINCUNX */
> + }
This reads poorly, which makes me think it's wrong. Is format supposed
to be a bitfield or an enum?
> +EXPORT_SYMBOL(drm_detect_3D_monitor);
I suspect this is poorly named. There exist 3D displays which are not
HDMI.
> +#define VSIF_RESET_INDEX 0xFFFFFFF8
> +#define VSIF_RESET_BIT_22 0xFFBFFFFF
> +#define VSIF_SET_BIT_19 0x80000
> +#define VSIF_RESET_BIT_20 0xFFEFFFFF
> +#define VSIF_RESET_BIT_17 0x10000
> +#define VSIF_SET_BIT_16 0xFFFDFFFF
> +#define VSIF_SET_MASTER_BIT 0x400000
i915 style is usually to write these as (1 << 22) I think.
More importantly, use semantic names for register contents. "Bit 17"
doesn't mean anything.
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 8020798..5b4d09c 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -798,6 +798,8 @@ extern int drm_mode_gamma_set_ioctl(struct drm_device
> *dev,
> extern u8 *drm_find_cea_extension(struct edid *edid);
> extern bool drm_detect_hdmi_monitor(struct edid *edid);
> extern bool drm_detect_monitor_audio(struct edid *edid);
> +extern bool drm_detect_3D_monitor(struct edid *edid);
> +//extern struct panel_3d_caps* drm_detect_3D_monitor(struct edid *edid);
Well, which is it?
> +enum s3d_formats {
> + FRAME_PACKING = 0x0,
> + FIELD_ALTERNATIVE = 0x1,
> + LINE_ALTERNATIVE = 0x2,
> + SIDE_BY_SIDE_FULL = 0x3,
> + L_DEPTH = 0x4,
> + L_DEPTH_GFX_GFX_DEPTH = 0x5,
> + TOP_BOTTOM = 0x6, /* 0x7 is reserved for future */
> + SIDE_BY_SIDE_HALF = 0x8 /* 0x9 onwards is reserved for future */
> +};
If format is supposed to be an enum, why aren't you using these symbolic
values?
- ajax
signature.asc
Description: This is a digitally signed message part
_______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
