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