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
Attachment:
signature.asc
Description: This is a digitally signed message part
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel