* Code stereo layouts as an enum in the mode structure
@ 2013-09-27 11:11 Damien Lespiau
2013-09-27 11:11 ` [PATCH 1/3] drm: Code stereo layouts as an enum rather than a bit field Damien Lespiau
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Damien Lespiau @ 2013-09-27 11:11 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
Daniel noticed that it wasn't very smart to keep using one bit per stereo
layout now that we don't have to or them. It's a nice final touch to the new
stereo mode API extension that we should fix before committing to it.
--
Damien
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/3] drm: Code stereo layouts as an enum rather than a bit field 2013-09-27 11:11 Code stereo layouts as an enum in the mode structure Damien Lespiau @ 2013-09-27 11:11 ` Damien Lespiau 2013-09-27 11:11 ` [PATCH 2/3] drm: Revert "drm: Reject modes with more than 1 stereo flags set" Damien Lespiau ` (2 subsequent siblings) 3 siblings, 0 replies; 6+ messages in thread From: Damien Lespiau @ 2013-09-27 11:11 UTC (permalink / raw) To: dri-devel; +Cc: intel-gfx This allows us to use fewer bits in the mode structure, leaving room for future work while allowing more stereo layouts types than we could have ever dreamt of. I also exposed the previously private DRM_MODE_FLAG_3D_MASK to set in stone that we are using 5 bits for the stereo layout enum, reserving 32 values. Even with that reservation, we gain 3 bits from the previous encoding. The code adding the mandatory stereo modes needeed to be adapted as it was relying or being able to or stereo layouts together. Suggested-by: Daniel Vetter <daniel@ffwll.ch> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> --- drivers/gpu/drm/drm_edid.c | 47 +++++++++++++++------------------------------ include/drm/drm_crtc.h | 9 --------- include/uapi/drm/drm_mode.h | 19 ++++++++++-------- 3 files changed, 26 insertions(+), 49 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index c24af1d..7d1e8a9 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2562,16 +2562,16 @@ struct stereo_mandatory_mode { }; static const struct stereo_mandatory_mode stereo_mandatory_modes[] = { - { 1920, 1080, 24, - DRM_MODE_FLAG_3D_TOP_AND_BOTTOM | DRM_MODE_FLAG_3D_FRAME_PACKING }, + { 1920, 1080, 24, DRM_MODE_FLAG_3D_TOP_AND_BOTTOM }, + { 1920, 1080, 24, DRM_MODE_FLAG_3D_FRAME_PACKING }, { 1920, 1080, 50, DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF }, { 1920, 1080, 60, DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF }, - { 1280, 720, 50, - DRM_MODE_FLAG_3D_TOP_AND_BOTTOM | DRM_MODE_FLAG_3D_FRAME_PACKING }, - { 1280, 720, 60, - DRM_MODE_FLAG_3D_TOP_AND_BOTTOM | DRM_MODE_FLAG_3D_FRAME_PACKING } + { 1280, 720, 50, DRM_MODE_FLAG_3D_TOP_AND_BOTTOM }, + { 1280, 720, 50, DRM_MODE_FLAG_3D_FRAME_PACKING }, + { 1280, 720, 60, DRM_MODE_FLAG_3D_TOP_AND_BOTTOM }, + { 1280, 720, 60, DRM_MODE_FLAG_3D_FRAME_PACKING } }; static bool @@ -2586,50 +2586,33 @@ stereo_match_mandatory(const struct drm_display_mode *mode, drm_mode_vrefresh(mode) == stereo_mode->vrefresh; } -static const struct stereo_mandatory_mode * -hdmi_find_stereo_mandatory_mode(const struct drm_display_mode *mode) -{ - int i; - - for (i = 0; i < ARRAY_SIZE(stereo_mandatory_modes); i++) - if (stereo_match_mandatory(mode, &stereo_mandatory_modes[i])) - return &stereo_mandatory_modes[i]; - - return NULL; -} - static int add_hdmi_mandatory_stereo_modes(struct drm_connector *connector) { struct drm_device *dev = connector->dev; const struct drm_display_mode *mode; struct list_head stereo_modes; - int modes = 0; + int modes = 0, i; INIT_LIST_HEAD(&stereo_modes); list_for_each_entry(mode, &connector->probed_modes, head) { - const struct stereo_mandatory_mode *mandatory; - u32 stereo_layouts, layout; - - mandatory = hdmi_find_stereo_mandatory_mode(mode); - if (!mandatory) - continue; - - stereo_layouts = mandatory->flags & DRM_MODE_FLAG_3D_MASK; - do { + for (i = 0; i < ARRAY_SIZE(stereo_mandatory_modes); i++) { + const struct stereo_mandatory_mode *mandatory; struct drm_display_mode *new_mode; - layout = 1 << (ffs(stereo_layouts) - 1); - stereo_layouts &= ~layout; + if (!stereo_match_mandatory(mode, + &stereo_mandatory_modes[i])) + continue; + mandatory = &stereo_mandatory_modes[i]; new_mode = drm_mode_duplicate(dev, mode); if (!new_mode) continue; - new_mode->flags |= layout; + new_mode->flags |= mandatory->flags; list_add_tail(&new_mode->head, &stereo_modes); modes++; - } while (stereo_layouts); + } } list_splice_tail(&stereo_modes, &connector->probed_modes); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index b2d08ca..eb6b8dc 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -181,15 +181,6 @@ struct drm_display_mode { int hsync; /* in kHz */ }; -#define DRM_MODE_FLAG_3D_MASK (DRM_MODE_FLAG_3D_FRAME_PACKING | \ - DRM_MODE_FLAG_3D_FIELD_ALTERNATIVE | \ - DRM_MODE_FLAG_3D_LINE_ALTERNATIVE | \ - DRM_MODE_FLAG_3D_SIDE_BY_SIDE_FULL | \ - DRM_MODE_FLAG_3D_L_DEPTH | \ - DRM_MODE_FLAG_3D_L_DEPTH_GFX_GFX_DEPTH | \ - DRM_MODE_FLAG_3D_TOP_AND_BOTTOM | \ - DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF) - static inline bool drm_mode_is_stereo(const struct drm_display_mode *mode) { return mode->flags & DRM_MODE_FLAG_3D_MASK; diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index bafe612..7980f89 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -58,14 +58,17 @@ #define DRM_MODE_FLAG_PIXMUX (1<<11) #define DRM_MODE_FLAG_DBLCLK (1<<12) #define DRM_MODE_FLAG_CLKDIV2 (1<<13) -#define DRM_MODE_FLAG_3D_FRAME_PACKING (1<<14) -#define DRM_MODE_FLAG_3D_FIELD_ALTERNATIVE (1<<15) -#define DRM_MODE_FLAG_3D_LINE_ALTERNATIVE (1<<16) -#define DRM_MODE_FLAG_3D_SIDE_BY_SIDE_FULL (1<<17) -#define DRM_MODE_FLAG_3D_L_DEPTH (1<<18) -#define DRM_MODE_FLAG_3D_L_DEPTH_GFX_GFX_DEPTH (1<<19) -#define DRM_MODE_FLAG_3D_TOP_AND_BOTTOM (1<<20) -#define DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF (1<<21) +#define DRM_MODE_FLAG_3D_MASK (0x1f<<14) +#define DRM_MODE_FLAG_3D_NONE (0<<14) +#define DRM_MODE_FLAG_3D_FRAME_PACKING (1<<14) +#define DRM_MODE_FLAG_3D_FIELD_ALTERNATIVE (2<<14) +#define DRM_MODE_FLAG_3D_LINE_ALTERNATIVE (3<<14) +#define DRM_MODE_FLAG_3D_SIDE_BY_SIDE_FULL (4<<14) +#define DRM_MODE_FLAG_3D_L_DEPTH (5<<14) +#define DRM_MODE_FLAG_3D_L_DEPTH_GFX_GFX_DEPTH (6<<14) +#define DRM_MODE_FLAG_3D_TOP_AND_BOTTOM (7<<14) +#define DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF (8<<14) + /* DPMS flags */ /* bit compatible with the xorg definitions. */ -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] drm: Revert "drm: Reject modes with more than 1 stereo flags set" 2013-09-27 11:11 Code stereo layouts as an enum in the mode structure Damien Lespiau 2013-09-27 11:11 ` [PATCH 1/3] drm: Code stereo layouts as an enum rather than a bit field Damien Lespiau @ 2013-09-27 11:11 ` Damien Lespiau 2013-09-27 11:11 ` [PATCH 3/3] drm: Reject stereo modes with an unknown layout Damien Lespiau 2013-09-27 15:36 ` Code stereo layouts as an enum in the mode structure Ville Syrjälä 3 siblings, 0 replies; 6+ messages in thread From: Damien Lespiau @ 2013-09-27 11:11 UTC (permalink / raw) To: dri-devel; +Cc: intel-gfx Now that the coding of stereo layout has changed from a bit field to an enum, we need remove that check. Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> --- drivers/gpu/drm/drm_crtc.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 807309f..2ce80ed 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1319,10 +1319,6 @@ static int drm_crtc_convert_umode(struct drm_display_mode *out, if (in->clock > INT_MAX || in->vrefresh > INT_MAX) return -ERANGE; - /* At most, 1 set bit describing the 3D layout of the mode */ - if (hweight32(in->flags & DRM_MODE_FLAG_3D_MASK) > 1) - return -EINVAL; - out->clock = in->clock; out->hdisplay = in->hdisplay; out->hsync_start = in->hsync_start; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] drm: Reject stereo modes with an unknown layout 2013-09-27 11:11 Code stereo layouts as an enum in the mode structure Damien Lespiau 2013-09-27 11:11 ` [PATCH 1/3] drm: Code stereo layouts as an enum rather than a bit field Damien Lespiau 2013-09-27 11:11 ` [PATCH 2/3] drm: Revert "drm: Reject modes with more than 1 stereo flags set" Damien Lespiau @ 2013-09-27 11:11 ` Damien Lespiau 2013-09-27 15:36 ` Code stereo layouts as an enum in the mode structure Ville Syrjälä 3 siblings, 0 replies; 6+ messages in thread From: Damien Lespiau @ 2013-09-27 11:11 UTC (permalink / raw) To: dri-devel; +Cc: intel-gfx The kernel shouldn't accept invalid modes, just say No. Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> --- drivers/gpu/drm/drm_crtc.c | 3 +++ include/drm/drm_crtc.h | 2 ++ include/uapi/drm/drm_mode.h | 4 ++++ 3 files changed, 9 insertions(+) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 2ce80ed..d7a8370 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1319,6 +1319,9 @@ static int drm_crtc_convert_umode(struct drm_display_mode *out, if (in->clock > INT_MAX || in->vrefresh > INT_MAX) return -ERANGE; + if ((in->flags & DRM_MODE_FLAG_3D_MASK) > DRM_MODE_FLAG_3D_MAX) + return -EINVAL; + out->clock = in->clock; out->hdisplay = in->hdisplay; out->hsync_start = in->hsync_start; diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index eb6b8dc..50cedad 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -128,6 +128,8 @@ enum drm_mode_status { #define CRTC_INTERLACE_HALVE_V (1 << 0) /* halve V values for interlacing */ #define CRTC_STEREO_DOUBLE (1 << 1) /* adjust timings for stereo modes */ +#define DRM_MODE_FLAG_3D_MAX DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF + struct drm_display_mode { /* Header */ struct list_head head; diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 7980f89..c2c4ace 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -58,6 +58,10 @@ #define DRM_MODE_FLAG_PIXMUX (1<<11) #define DRM_MODE_FLAG_DBLCLK (1<<12) #define DRM_MODE_FLAG_CLKDIV2 (1<<13) + /* + * When adding a new stereo mode don't forget to adjust DRM_MODE_FLAGS_3D_MAX + * (define not exposed to user space). + */ #define DRM_MODE_FLAG_3D_MASK (0x1f<<14) #define DRM_MODE_FLAG_3D_NONE (0<<14) #define DRM_MODE_FLAG_3D_FRAME_PACKING (1<<14) -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: Code stereo layouts as an enum in the mode structure 2013-09-27 11:11 Code stereo layouts as an enum in the mode structure Damien Lespiau ` (2 preceding siblings ...) 2013-09-27 11:11 ` [PATCH 3/3] drm: Reject stereo modes with an unknown layout Damien Lespiau @ 2013-09-27 15:36 ` Ville Syrjälä 2013-09-27 19:41 ` Daniel Vetter 3 siblings, 1 reply; 6+ messages in thread From: Ville Syrjälä @ 2013-09-27 15:36 UTC (permalink / raw) To: Damien Lespiau; +Cc: intel-gfx, dri-devel On Fri, Sep 27, 2013 at 12:11:47PM +0100, Damien Lespiau wrote: > Daniel noticed that it wasn't very smart to keep using one bit per stereo > layout now that we don't have to or them. It's a nice final touch to the new > stereo mode API extension that we should fix before committing to it. Assuming you still trust me even though I didn't see this possibility during my review of the original series :) For the series: Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> -- Ville Syrjälä Intel OTC ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Code stereo layouts as an enum in the mode structure 2013-09-27 15:36 ` Code stereo layouts as an enum in the mode structure Ville Syrjälä @ 2013-09-27 19:41 ` Daniel Vetter 0 siblings, 0 replies; 6+ messages in thread From: Daniel Vetter @ 2013-09-27 19:41 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx, dri-devel On Fri, Sep 27, 2013 at 06:36:05PM +0300, Ville Syrjälä wrote: > On Fri, Sep 27, 2013 at 12:11:47PM +0100, Damien Lespiau wrote: > > Daniel noticed that it wasn't very smart to keep using one bit per stereo > > layout now that we don't have to or them. It's a nice final touch to the new > > stereo mode API extension that we should fix before committing to it. > > Assuming you still trust me even though I didn't see this possibility > during my review of the original series :) > > For the series: > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Merged to dinq, thansk for patches&review. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-09-27 19:40 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-09-27 11:11 Code stereo layouts as an enum in the mode structure Damien Lespiau 2013-09-27 11:11 ` [PATCH 1/3] drm: Code stereo layouts as an enum rather than a bit field Damien Lespiau 2013-09-27 11:11 ` [PATCH 2/3] drm: Revert "drm: Reject modes with more than 1 stereo flags set" Damien Lespiau 2013-09-27 11:11 ` [PATCH 3/3] drm: Reject stereo modes with an unknown layout Damien Lespiau 2013-09-27 15:36 ` Code stereo layouts as an enum in the mode structure Ville Syrjälä 2013-09-27 19:41 ` Daniel Vetter
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.