[Intel-gfx] Enabling all display interfaces
One of the latest "VLV" patch-sets is from Vijay Purushothaman for enabling all display interfaces. http://phoronix.com/forums/showthread.php?74010-Intel-Valley-View-Support-Continues-To-Mature&p=288947#post288947 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] could you stop this spam
On Sáb, 2012-09-29 at 00:36 +0200, Paul Menzel wrote: > Am Freitag, den 28.09.2012, 18:00 +0100 schrieb Sérgio Basto: > > This spam is making my spam filter get crazy . > > Never paste spam! yeah , sorry never though about that, thanks for advise -- Sérgio M. B. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Call drm_handle_vblank() after processing pending pageflips
Reviewed-by: Rodrigo Vivi
On Fri, Sep 28, 2012 at 8:25 AM, Chris Wilson wrote:
> When we process a pageflip completion, we append an event to the vblank
> queue. That queue is processed by drm_handle_vblank() and so by calling
> drm_handle_vblank() prior to processing the pageflips, we incur an extra
> frame of latency in reporting the flip event.
>
> Signed-off-by: Chris Wilson
> ---
> drivers/gpu/drm/i915/i915_irq.c | 22 ++
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 6361276..e18e56b 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -564,13 +564,12 @@ static irqreturn_t valleyview_irq_handler(DRM_IRQ_ARGS)
> spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>
> for_each_pipe(pipe) {
> - if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS)
> - drm_handle_vblank(dev, pipe);
> -
> if (pipe_stats[pipe] & PLANE_FLIPDONE_INT_STATUS_VLV)
> {
> intel_prepare_page_flip(dev, pipe);
> intel_finish_page_flip(dev, pipe);
> }
> + if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS)
> + drm_handle_vblank(dev, pipe);
> }
>
> /* Consume port. Then clear IIR or we'll miss events */
> @@ -2220,22 +2219,22 @@ static irqreturn_t i8xx_irq_handler(DRM_IRQ_ARGS)
> if (iir & I915_USER_INTERRUPT)
> notify_ring(dev, &dev_priv->ring[RCS]);
>
> - if (pipe_stats[0] & PIPE_VBLANK_INTERRUPT_STATUS &&
> - drm_handle_vblank(dev, 0)) {
> + if (pipe_stats[0] & PIPE_VBLANK_INTERRUPT_STATUS) {
> if (iir &
> I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT) {
> intel_prepare_page_flip(dev, 0);
> intel_finish_page_flip(dev, 0);
> flip_mask &=
> ~I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT;
> }
> + drm_handle_vblank(dev, 0);
> }
>
> - if (pipe_stats[1] & PIPE_VBLANK_INTERRUPT_STATUS &&
> - drm_handle_vblank(dev, 1)) {
> + if (pipe_stats[1] & PIPE_VBLANK_INTERRUPT_STATUS) {
> if (iir &
> I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT) {
> intel_prepare_page_flip(dev, 1);
> intel_finish_page_flip(dev, 1);
> flip_mask &=
> ~I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT;
> }
> + drm_handle_vblank(dev, 1);
> }
>
> iir = new_iir;
> @@ -2418,13 +2417,13 @@ static irqreturn_t i915_irq_handler(DRM_IRQ_ARGS)
> int plane = pipe;
> if (IS_MOBILE(dev))
> plane = !plane;
> - if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS &&
> - drm_handle_vblank(dev, pipe)) {
> + if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS) {
> if (iir & flip[plane]) {
> intel_prepare_page_flip(dev, plane);
> intel_finish_page_flip(dev, pipe);
> flip_mask &= ~flip[plane];
> }
> + drm_handle_vblank(dev, pipe);
> }
>
> if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
> @@ -2662,17 +2661,16 @@ static irqreturn_t i965_irq_handler(DRM_IRQ_ARGS)
> intel_prepare_page_flip(dev, 1);
>
> for_each_pipe(pipe) {
> - if (pipe_stats[pipe] &
> PIPE_START_VBLANK_INTERRUPT_STATUS &&
> - drm_handle_vblank(dev, pipe)) {
> + if (pipe_stats[pipe] &
> PIPE_START_VBLANK_INTERRUPT_STATUS) {
> i915_pageflip_stall_check(dev, pipe);
> intel_finish_page_flip(dev, pipe);
> + drm_handle_vblank(dev, pipe);
> }
>
> if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
> blc_event = true;
> }
>
> -
> if (blc_event || (iir & I915_ASLE_INTERRUPT))
> intel_opregion_asle_intr(dev);
>
> --
> 1.7.10.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedes
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Add HDMI vendor info frame support
On Thu, Sep 27, 2012 at 3:41 PM, Damien Lespiau
wrote:
> From: Damien Lespiau
>
> When scanning out a 3D framebuffer, send the corresponding infoframe to
> the HDMI sink.
>
> See http://www.hdmi.org/manufacturer/specification.aspx for details.
>
> Signed-off-by: Damien Lespiau
> ---
> drivers/gpu/drm/i915/intel_drv.h | 14 +++
> drivers/gpu/drm/i915/intel_hdmi.c | 49
> +-
> drivers/gpu/drm/i915/intel_modes.c | 12 ++
> 3 files changed, 74 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index cd54cf8..c326d30e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -263,6 +263,13 @@ struct cxsr_latency {
> #define DIP_SPD_BD 0xa
> #define DIP_SPD_SCD0xb
>
> +#define DIP_TYPE_VENDOR0x81
> +#define DIP_VERSION_VENDOR 0x1
> +#define DIP_HDMI_3D_PRESENT(0x2<<5)
> +#define DIP_HDMI_3D_STRUCT_FP (0x0<<4)
> +#define DIP_HDMI_3D_STRUCT_TB (0x6<<4)
> +#define DIP_HDMI_3D_STRUCT_SBSH(0x8<<4)
> +
> struct dip_infoframe {
> uint8_t type; /* HB0 */
> uint8_t ver;/* HB1 */
> @@ -292,6 +299,12 @@ struct dip_infoframe {
> uint8_t pd[16];
> uint8_t sdi;
> } __attribute__ ((packed)) spd;
> + struct {
> + uint8_t vendor_id[3];
> + uint8_t video_format;
> + uint8_t s3d_struct;
> + uint8_t s3d_ext_data;
> + } __attribute__ ((packed)) hdmi;
> uint8_t payload[27];
> } __attribute__ ((packed)) body;
> } __attribute__((packed));
> @@ -348,6 +361,7 @@ int intel_ddc_get_modes(struct drm_connector *c, struct
> i2c_adapter *adapter);
>
> extern void intel_attach_force_audio_property(struct drm_connector
> *connector);
> extern void intel_attach_broadcast_rgb_property(struct drm_connector
> *connector);
> +extern void intel_attach_expose_3d_modes_property(struct drm_connector
> *connector);
>
> extern void intel_crt_init(struct drm_device *dev);
> extern void intel_hdmi_init(struct drm_device *dev, int sdvox_reg);
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 98f6024..2d51b7e 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -83,6 +83,8 @@ static u32 g4x_infoframe_index(struct dip_infoframe *frame)
> return VIDEO_DIP_SELECT_AVI;
> case DIP_TYPE_SPD:
> return VIDEO_DIP_SELECT_SPD;
> + case DIP_TYPE_VENDOR:
> + return VIDEO_DIP_SELECT_VENDOR;
> default:
> DRM_DEBUG_DRIVER("unknown info frame type %d\n", frame->type);
> return 0;
> @@ -96,6 +98,8 @@ static u32 g4x_infoframe_enable(struct dip_infoframe *frame)
> return VIDEO_DIP_ENABLE_AVI;
> case DIP_TYPE_SPD:
> return VIDEO_DIP_ENABLE_SPD;
> + case DIP_TYPE_VENDOR:
> + return VIDEO_DIP_ENABLE_VENDOR;
> default:
> DRM_DEBUG_DRIVER("unknown info frame type %d\n", frame->type);
> return 0;
> @@ -338,6 +342,42 @@ static void intel_hdmi_set_spd_infoframe(struct
> drm_encoder *encoder)
> intel_set_infoframe(encoder, &spd_if);
> }
>
> +static void intel_hdmi_set_hdmi_infoframe(struct drm_encoder *encoder,
> + struct drm_display_mode
> *adjusted_mode)
> +{
> + struct dip_infoframe hdmi_if;
> +
> + /* We really only need to send a HDMI vendor info frame when having
> +* a 3D format to describe */
> + if (!(adjusted_mode->flags & DRM_MODE_FLAG_3D_MASK))
> + return;
> +
> + memset(&hdmi_if, 0, sizeof(hdmi_if));
> + hdmi_if.type = DIP_TYPE_VENDOR;
> + hdmi_if.ver = DIP_VERSION_VENDOR;
> + /* HDMI IEEE registration id, least significant bit first */
> + hdmi_if.body.hdmi.vendor_id[0] = 0x03;
> + hdmi_if.body.hdmi.vendor_id[1] = 0x0c;
> + hdmi_if.body.hdmi.vendor_id[2] = 0x00;
> + hdmi_if.body.hdmi.video_format = DIP_HDMI_3D_PRESENT;
> + if (adjusted_mode->flags & DRM_MODE_FLAG_3D_FRAME_PACKING)
> + hdmi_if.body.hdmi.s3d_struct = DIP_HDMI_3D_STRUCT_FP;
> + else if (adjusted_mode->flags & DRM_MODE_FLAG_3D_TOP_BOTTOM)
> + hdmi_if.body.hdmi.s3d_struct = DIP_HDMI_3D_STRUCT_TB;
> + else if (adjusted_mode->flags & DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF)
> + hdmi_if.body.hdmi.s3d_struct = DIP_HDMI_3D_STRUCT_SBSH;
> + /* len is the payload len, not including checksum. Side by side (half)
> +* has an extra byte for 3D_Ext_Data */
> + if (adjusted_mode->flags & DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF) {
> + hdmi_if.len = 6;
> + /*
Re: [Intel-gfx] [PATCH 2/3] drm: Parse the HDMI cea vendor block for 3D present
Reviewed-by: Rodrigo Vivi
Tested-by: Rodrigo Vivi
On Thu, Sep 27, 2012 at 3:41 PM, Damien Lespiau
wrote:
> From: Damien Lespiau
>
> For now, let's just look at the 3D_present flag of the CEA HDMI vendor
> block to detect if the sink supports a small list of then mandatory 3D
> formats.
>
> See the HDMI 1.4a 3D extraction for detail:
> http://www.hdmi.org/manufacturer/specification.aspx
>
> Signed-off-by: Damien Lespiau
> ---
> drivers/gpu/drm/drm_edid.c | 87
> --
> include/drm/drm_mode.h | 35 +++
> 2 files changed, 105 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index b7ee230..7eecfa0 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1522,21 +1522,102 @@ do_cea_modes (struct drm_connector *connector, u8
> *db, u8 len)
> return modes;
> }
>
> +static bool cea_hdmi_3d_present(u8 *hdmi)
> +{
> + u8 len, skip = 0;
> +
> + len = hdmi[0] & 0x1f;
> +
> + if (len < 8)
> + return false;
> +
> + /* no HDMI_Video_present */
> + if (!(hdmi[8] & (1<<5)))
> + return false;
> +
> + /* Latency_fields_present */
> + if (hdmi[8] & (1 << 7))
> + skip += 2;
> +
> + /* I_Latency_fields_present */
> + if (hdmi[8] & (1 << 6))
> + skip += 2;
> +
> + /* the declared length is not long enough */
> + if (len < (9 + skip))
> + return false;
> +
> + return (hdmi[9 + skip] & (1 << 7)) != 0;
> +}
> +
> +static const struct {
> + int width, height, freq;
> + unsigned int select, value;
> + unsigned int formats;
> +} s3d_mandatory_modes[] = {
> + { 1920, 1080, 24, DRM_MODE_FLAG_INTERLACE, 0,
> + DRM_MODE_FLAG_3D_TOP_BOTTOM | DRM_MODE_FLAG_3D_FRAME_PACKING },
> + { 1920, 1080, 50, DRM_MODE_FLAG_INTERLACE, DRM_MODE_FLAG_INTERLACE,
> + DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF },
> + { 1920, 1080, 60, DRM_MODE_FLAG_INTERLACE, DRM_MODE_FLAG_INTERLACE,
> + DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF },
> + { 1280, 720, 50, DRM_MODE_FLAG_INTERLACE, 0,
> + DRM_MODE_FLAG_3D_TOP_BOTTOM | DRM_MODE_FLAG_3D_FRAME_PACKING },
> + { 1280, 720, 60, DRM_MODE_FLAG_INTERLACE, 0,
> + DRM_MODE_FLAG_3D_TOP_BOTTOM | DRM_MODE_FLAG_3D_FRAME_PACKING }
> +};
> +
> +static void cea_hdmi_patch_mandatory_3d_mode(struct drm_display_mode *mode)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(s3d_mandatory_modes); i++) {
> + if (mode->hdisplay == s3d_mandatory_modes[i].width &&
> + mode->vdisplay == s3d_mandatory_modes[i].height &&
> + (mode->flags & s3d_mandatory_modes[i].select) ==
> + s3d_mandatory_modes[i].value &&
> + drm_mode_vrefresh(mode) == s3d_mandatory_modes[i].freq) {
> + mode->flags |= s3d_mandatory_modes[i].formats;
> + }
> + }
> +}
> +
> +static void cea_hdmi_patch_mandatory_3d_modes(struct drm_connector
> *connector)
> +{
> + struct drm_display_mode *mode;
> +
> + list_for_each_entry(mode, &connector->probed_modes, head)
> + cea_hdmi_patch_mandatory_3d_mode(mode);
> +}
> +
> static int
> add_cea_modes(struct drm_connector *connector, struct edid *edid)
> {
> u8 * cea = drm_find_cea_extension(edid);
> - u8 * db, dbl;
> - int modes = 0;
> + u8 * db, *hdmi = NULL, dbl;
> + int modes = 0, vendor_id;
>
> + /* let's find the cea modes before looking at the hdmi vendor block
> +* as the 3d_present flag needs to know about the supported modes
> +* to infer the 3D modes */
> if (cea && cea[1] >= 3) {
> for (db = cea + 4; db < cea + cea[2]; db += dbl + 1) {
> dbl = db[0] & 0x1f;
> - if (((db[0] & 0xe0) >> 5) == VIDEO_BLOCK)
> + switch ((db[0] & 0xe0) >> 5) {
> + case VIDEO_BLOCK:
> modes += do_cea_modes (connector, db+1, dbl);
> + break;
> + case VENDOR_BLOCK:
> + vendor_id = db[1] | db[2] << 8 | db[3] << 16;
> + if (vendor_id == HDMI_IDENTIFIER)
> + hdmi = db;
> + }
> }
> }
>
> + if (connector->expose_3d_modes && hdmi && cea_hdmi_3d_present(hdmi))
> + cea_hdmi_patch_mandatory_3d_modes(connector);
> +
> return modes;
> }
>
> diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
> index 45b19c6..d5d22de 100644
> --- a/include/drm/drm_mode.h
> +++ b/include/drm/drm_mode.h
> @@ -44,20 +44,27 @@
>
> /* Video mode flags */
> /* bit compatible with the xorg definitions. */
>
Re: [Intel-gfx] [PATCH 1/2] lib: Dump information about the supported 3D stereo formats
Reviewed-by: Rodrigo Vivi
Tested-by: Rodrigo Vivi
On Thu, Sep 27, 2012 at 3:42 PM, Damien Lespiau
wrote:
> From: Damien Lespiau
>
> When dumping the details of a mode, let's add the 3D formats the mode
> supports.
>
> Signed-off-by: Damien Lespiau
> ---
> lib/drmtest.c | 13 +++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index 8df9797..4d5a67c 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -812,7 +812,15 @@ unsigned int kmstest_create_fb(int fd, int width, int
> height, int bpp,
>
> void kmstest_dump_mode(drmModeModeInfo *mode)
> {
> - printf(" %s %d %d %d %d %d %d %d %d %d 0x%x 0x%x %d\n",
> + bool stereo_3d = mode->flags & DRM_MODE_FLAG_3D_MASK;
> + char flags_str[32];
> +
> + snprintf(flags_str, sizeof(flags_str), " (3D:%s%s%s)",
> +mode->flags & DRM_MODE_FLAG_3D_TOP_BOTTOM ? " TB": "",
> +mode->flags & DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF ? " SBSH":
> "",
> +mode->flags & DRM_MODE_FLAG_3D_FRAME_PACKING ? " FP": "");
> +
> + printf(" %s %d %d %d %d %d %d %d %d %d 0x%x 0x%x %d%s\n",
>mode->name,
>mode->vrefresh,
>mode->hdisplay,
> @@ -825,7 +833,8 @@ void kmstest_dump_mode(drmModeModeInfo *mode)
>mode->vtotal,
>mode->flags,
>mode->type,
> - mode->clock);
> + mode->clock,
> + stereo_3d ? flags_str : "");
> fflush(stdout);
> }
>
> --
> 1.7.11.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm: Add an "expose 3d modes" property
Reviewed-by: Rodrigo Vivi
Tested-by: Rodrigo Vivi
On Thu, Sep 27, 2012 at 3:41 PM, Damien Lespiau
wrote:
> From: Damien Lespiau
>
> The "expose 3D modes" property can be attached to connectors to allow
> user space to indicate it can deal with 3D modes and that the drm driver
> should expose those 3D modes.
>
> Signed-off-by: Damien Lespiau
> ---
> drivers/gpu/drm/drm_crtc.c | 35 ++-
> include/drm/drm_crtc.h | 6 ++
> include/drm/drm_mode.h | 4
> 3 files changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 6fbfc24..10a289c 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -841,6 +841,35 @@ int drm_mode_create_tv_properties(struct drm_device
> *dev, int num_modes,
> }
> EXPORT_SYMBOL(drm_mode_create_tv_properties);
>
> +static const struct drm_prop_enum_list expose_3d_modes_list[] =
> +{
> + { DRM_MODE_EXPOSE_3D_MODES_OFF, "Off" },
> + { DRM_MODE_EXPOSE_3D_MODES_ON, "On" }
> +};
> +
> +/**
> + * drm_mode_create_s3d_properties - create stereo 3D properties
> + * @dev: DRM device
> + *
> + * This functions create properties that are used by the stereo 3D code.
> Those
> + * properties must be attached to the desired connectors afterwards.
> + */
> +int drm_mode_create_s3d_properties(struct drm_device *dev)
> +{
> + struct drm_property *prop;
> +
> + if (dev->mode_config.s3d_expose_modes_property)
> + return 0;
> +
> + prop = drm_property_create_enum(dev, 0, "expose 3D modes",
> + expose_3d_modes_list,
> + ARRAY_SIZE(expose_3d_modes_list));
> + dev->mode_config.s3d_expose_modes_property = prop;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_mode_create_s3d_properties);
> +
> /**
> * drm_mode_create_scaling_mode_property - create scaling mode property
> * @dev: DRM device
> @@ -3170,12 +3199,16 @@ static int drm_mode_connector_set_obj_prop(struct
> drm_mode_object *obj,
> {
> int ret = -EINVAL;
> struct drm_connector *connector = obj_to_connector(obj);
> + struct drm_device *dev = connector->dev;
>
> /* Do DPMS ourselves */
> - if (property == connector->dev->mode_config.dpms_property) {
> + if (property == dev->mode_config.dpms_property) {
> if (connector->funcs->dpms)
> (*connector->funcs->dpms)(connector, (int)value);
> ret = 0;
> + } else if (property == dev->mode_config.s3d_expose_modes_property) {
> + connector->expose_3d_modes = value;
> + ret = 0;
> } else if (connector->funcs->set_property)
> ret = connector->funcs->set_property(connector, property,
> value);
>
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index bfacf0d..34372cb 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -578,6 +578,8 @@ struct drm_connector {
> /* requested DPMS state */
> int dpms;
>
> + int expose_3d_modes;
> +
> void *helper_private;
>
> /* forced on connector */
> @@ -802,6 +804,9 @@ struct drm_mode_config {
> struct drm_property *tv_saturation_property;
> struct drm_property *tv_hue_property;
>
> + /* Stereo 3D properties */
> + struct drm_property *s3d_expose_modes_property;
> +
> /* Optional properties */
> struct drm_property *scaling_mode_property;
> struct drm_property *dithering_mode_property;
> @@ -950,6 +955,7 @@ extern int drm_property_add_enum(struct drm_property
> *property, int index,
> extern int drm_mode_create_dvi_i_properties(struct drm_device *dev);
> extern int drm_mode_create_tv_properties(struct drm_device *dev, int
> num_formats,
> char *formats[]);
> +extern int drm_mode_create_s3d_properties(struct drm_device *dev);
> extern int drm_mode_create_scaling_mode_property(struct drm_device *dev);
> extern int drm_mode_create_dithering_property(struct drm_device *dev);
> extern int drm_mode_create_dirty_info_property(struct drm_device *dev);
> diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
> index 3d6301b..45b19c6 100644
> --- a/include/drm/drm_mode.h
> +++ b/include/drm/drm_mode.h
> @@ -83,6 +83,10 @@
> #define DRM_MODE_DIRTY_ON 1
> #define DRM_MODE_DIRTY_ANNOTATE 2
>
> +/* Expose 3D modes */
> +#define DRM_MODE_EXPOSE_3D_MODES_OFF 0
> +#define DRM_MODE_EXPOSE_3D_MODES_ON1
> +
> struct drm_mode_modeinfo {
> __u32 clock;
> __u16 hdisplay, hsync_start, hsync_end, htotal, hskew;
> --
> 1.7.11.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_
Re: [Intel-gfx] [PATCH 2/2] tests/testdisplay: Test the stereo 3D modes
On Thu, Sep 27, 2012 at 3:42 PM, Damien Lespiau
wrote:
> From: Damien Lespiau
>
> Now that modes have flags to describe which 3d formats the sink
> supports, it's time to test them.
>
> The new test cycles through the supported 3D formats and paint 3D
> stereoscopic images taken from publicly available samples:
> http://www.quantumdata.com/apps/3D/sample_BMP.asp
>
> Signed-off-by: Damien Lespiau
> ---
> tests/testdisplay.c | 226
> ++--
> 1 file changed, 221 insertions(+), 5 deletions(-)
>
> diff --git a/tests/testdisplay.c b/tests/testdisplay.c
> index c52bb2f..e179c83 100644
> --- a/tests/testdisplay.c
> +++ b/tests/testdisplay.c
> @@ -52,6 +52,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -68,7 +69,7 @@
> drmModeRes *resources;
> int drm_fd, modes;
> int dump_info = 0, test_all_modes =0, test_preferred_mode = 0, force_mode =
> 0,
> - test_plane, enable_tiling;
> + test_plane, test_3d_modes, enable_tiling;
> int sleep_between_modes = 5;
> uint32_t depth = 24, stride, bpp;
> int qr_code = 0;
> @@ -153,8 +154,51 @@ struct connector {
> drmModeConnector *connector;
> int crtc;
> int pipe;
> +
> + /* stereo 3d */
> + int s3d_format;
> + char s3d_image[32];
> };
>
> +static bool connector_expose_3d(uint32_t connector_id, bool enable)
> +{
> + drmModeConnector *connector;
> + drmModePropertyRes *property;
> + bool status = false;
> + int i;
> +
> + connector = drmModeGetConnector(drm_fd, connector_id);
> + if (connector->count_props == 0)
> + return false;
> +
> + for (i = 0; i < connector->count_props; i++) {
> + property = drmModeGetProperty(drm_fd, connector->props[i]);
> + if (!property)
> + continue;
> +
> + if (strcmp(property->name, "expose 3D modes") == 0) {
> + if (drmModeConnectorSetProperty(drm_fd,
> + connector_id,
> + property->prop_id,
> + enable))
> + fprintf(stderr, "failed to set the \"expose
> 3D "
> + "modes\" property on connector %d:
> %s\n",
> + connector_id, strerror(errno));
> + else
> + status = true;
> + drmModeFreeProperty(property);
> + goto out;
> + }
> +
> + drmModeFreeProperty(property);
> + property = NULL;
> + }
> +
> +out:
> + drmModeFreeConnector(connector);
> + return status;
> +}
> +
> static void dump_connectors_fd(int drmfd)
> {
> int i, j;
> @@ -172,11 +216,13 @@ static void dump_connectors_fd(int drmfd)
> for (i = 0; i < mode_resources->count_connectors; i++) {
> drmModeConnector *connector;
>
> + connector_expose_3d(mode_resources->connectors[i], TRUE);
> +
> connector = drmModeGetConnector(drmfd,
> mode_resources->connectors[i]);
> if (!connector) {
> fprintf(stderr, "could not get connector %i: %s\n",
> mode_resources->connectors[i],
> strerror(errno));
> - continue;
> + goto next;
> }
>
> printf("%d\t%d\t%s\t%s\t%dx%d\t\t%d\n",
> @@ -188,7 +234,7 @@ static void dump_connectors_fd(int drmfd)
>connector->count_modes);
>
> if (!connector->count_modes)
> - continue;
> + goto next;
>
> printf(" modes:\n");
> printf(" name refresh (Hz) hdisp hss hse htot vdisp "
> @@ -197,6 +243,9 @@ static void dump_connectors_fd(int drmfd)
> kmstest_dump_mode(&connector->modes[j]);
>
> drmModeFreeConnector(connector);
> +
> +next:
> + connector_expose_3d(mode_resources->connectors[i], FALSE);
> }
> printf("\n");
>
> @@ -554,6 +603,154 @@ set_mode(struct connector *c)
> drmModeFreeConnector(c->connector);
> }
>
> +static void
> +paint_3d_image(cairo_t *cr, int l_width, int l_height, void *priv)
> +{
> + struct connector *c = priv;
> + cairo_surface_t *image;
> +
> + image = cairo_image_surface_create_from_png(c->s3d_image);
> +
> + cairo_set_source_surface(cr, image, 0, 0);
> + cairo_paint(cr);
> +
> + cairo_surface_destroy(image);
> +}
> +
> +static void adjust_3d_timings(drmModeModeInfo *mode, unsigned int format)
> +{
> + uint16_t vdisplay, vactive_space;
> +
> + /* just set the 3D format we are setting (this
Re: [Intel-gfx] could you stop this spam
Am Freitag, den 28.09.2012, 18:00 +0100 schrieb Sérgio Basto: > This spam is making my spam filter get crazy . Never paste spam! Thanks, Paul signature.asc Description: This is a digitally signed message part ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] could you stop this spam Re: This company is making noise
This spam is making my spam filter get crazy . On Qui, 2012-09-27 at 22:49 -0800, Sampson Vinson wrote: > Advice: Purchase/Stay > This stock is cheap > > Date: Friday, Sep 28th > Name: FaceUp Entertainment Group Inc > Ticker: F_U E_G > Closed yesterday: $.28 > Target Price: $1.47 > > Ruling of Federal Judge Weinstein That Internet poker is a game of Skill. > This stock is in a sweet spot. > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Sérgio M. B. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 7/9] drm/i915: Add eDP support for Valleyview
On Thu, Sep 27, 2012 at 08:24:46AM -0700, Jesse Barnes wrote:
> On Thu, 27 Sep 2012 19:13:07 +0530
> Vijay Purushothaman wrote:
>
> > From: Gajanan Bhat
> >
> > Eventhough Valleyview display block is derived from Cantiga, VLV
> > supports eDP. So, added eDP checks in i9xx_crtc_mode_set path.
> >
> > v2: use different DPIO_DIVISOR values for VGA, DP and eDP
> > v3: fix DPIO value calculation to use same values for all display
> > interfaces
> > v4: removed unconditional enabling of 6bpc dithering based on comments
> > from Daniel & Jani Nikula. Also changed the display enabling order to
> > force eDP detection first.
> >
> > Signed-off-by: Gajanan Bhat
> > Signed-off-by: Vijay Purushothaman
Ok, I've slurped in the patches with the exception of the vga detect quirk
(maybe that'll go away with the hpd rework anyway) and the panel stuff
(since no one volunteered yet to smash an r-b onto it).
For the edp stuff here I think we need to add a bit more abstraction
eventually - maybe add an enum of the different kinds of DP connectors
(cpu, fdi, gmch) or add some vtable interfaces for the things which are
different on different platforms/ports ...
Thanks, Daniel
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 15 ---
> > drivers/gpu/drm/i915/intel_dp.c | 17 -
> > 2 files changed, 24 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index ed749c4..0362c80 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4413,6 +4413,14 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
> > }
> > }
> >
> > + if (IS_VALLEYVIEW(dev) && intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP)) {
> > + if (adjusted_mode->private_flags & INTEL_MODE_DP_FORCE_6BPC) {
> > + pipeconf |= PIPECONF_BPP_6 |
> > + PIPECONF_ENABLE |
> > + I965_PIPECONF_ACTIVE;
> > + }
> > + }
> > +
> > DRM_DEBUG_KMS("Mode for pipe %c:\n", pipe == 0 ? 'A' : 'B');
> > drm_mode_debug_printmodeline(mode);
> >
> > @@ -7623,6 +7631,10 @@ static void intel_setup_outputs(struct drm_device
> > *dev)
> > } else if (IS_VALLEYVIEW(dev)) {
> > int found;
> >
> > + /* Check for built-in panel first. Shares lanes with HDMI on
> > SDVOC */
> > + if (I915_READ(DP_C) & DP_DETECTED)
> > + intel_dp_init(dev, DP_C, PORT_C);
> > +
> > if (I915_READ(SDVOB) & PORT_DETECTED) {
> > /* SDVOB multiplex with HDMIB */
> > found = intel_sdvo_init(dev, SDVOB, true);
> > @@ -7635,9 +7647,6 @@ static void intel_setup_outputs(struct drm_device
> > *dev)
> > if (I915_READ(SDVOC) & PORT_DETECTED)
> > intel_hdmi_init(dev, SDVOC, PORT_C);
> >
> > - /* Shares lanes with HDMI on SDVOC */
> > - if (I915_READ(DP_C) & DP_DETECTED)
> > - intel_dp_init(dev, DP_C, PORT_C);
> > } else if (SUPPORTS_DIGITAL_OUTPUTS(dev)) {
> > bool found = false;
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index c111c3f..867c568 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -885,7 +885,7 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct
> > drm_display_mode *mode,
> >
> > /* Split out the IBX/CPU vs CPT settings */
> >
> > - if (is_cpu_edp(intel_dp) && IS_GEN7(dev)) {
> > + if (is_cpu_edp(intel_dp) && IS_GEN7(dev) && !IS_VALLEYVIEW(dev)) {
> > if (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC)
> > intel_dp->DP |= DP_SYNC_HS_HIGH;
> > if (adjusted_mode->flags & DRM_MODE_FLAG_PVSYNC)
> > @@ -1474,7 +1474,7 @@ intel_dp_pre_emphasis_max(struct intel_dp *intel_dp,
> > uint8_t voltage_swing)
> > {
> > struct drm_device *dev = intel_dp->base.base.dev;
> >
> > - if (IS_GEN7(dev) && is_cpu_edp(intel_dp)) {
> > + if (IS_GEN7(dev) && is_cpu_edp(intel_dp) && !IS_VALLEYVIEW(dev)) {
> > switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> > case DP_TRAIN_VOLTAGE_SWING_400:
> > return DP_TRAIN_PRE_EMPHASIS_6;
> > @@ -1773,7 +1773,7 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
> > uint32_tsignal_levels;
> >
> >
> > - if (IS_GEN7(dev) && is_cpu_edp(intel_dp)) {
> > + if (IS_GEN7(dev) && is_cpu_edp(intel_dp) &&
> > !IS_VALLEYVIEW(dev)) {
> > signal_levels =
> > intel_gen7_edp_signal_levels(intel_dp->train_set[0]);
> > DP = (DP & ~EDP_LINK_TRAIN_VOL_EMP_MASK_IVB) |
> > signal_levels;
> > } else if (IS_GEN6(dev) && is_cpu_edp(intel_dp)) {
> > @@ -1859,7 +1859,7 @@ intel_dp_complete_link_train(struct inte
Re: [Intel-gfx] [PATCH v2 6/9] drm/i915: Enable DisplayPort in Valleyview
On Thu, Sep 27, 2012 at 08:23:37AM -0700, Jesse Barnes wrote:
> On Thu, 27 Sep 2012 19:13:06 +0530
> Vijay Purushothaman wrote:
>
> > In valleyview voltageswing, pre-emphasis and lane control registers can
> > be programmed only through the h/w side band fabric.
> >
> > Cleaned up DPLL calculations for Valleyview to support multi display
> > configurations.
> >
> > v2: Based on Daniel's feedbacak, moved crt hotplug detect work around as
> > separate
> > patch. Also moved i9xx_update_pll_dividers to i8xx_update_pll and
> > i9xx_update_pll.
> >
> > Signed-off-by: Vijay Purushothaman
> > Signed-off-by: Gajanan Bhat
Small bikeshed: I've killed some spurious whitespace changs while
applying ...
-Daniel
> > ---
> > drivers/gpu/drm/i915/i915_reg.h |8 +--
> > drivers/gpu/drm/i915/intel_display.c | 90
> > --
> > 2 files changed, 66 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 3f75ee6..0fe4aad 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -385,12 +385,8 @@
> >
> > #define DPIO_FASTCLK_DISABLE 0x8100
> >
> > -#define _DPIO_DATA_LANE0 0x0220
> > -#define _DPIO_DATA_LANE1 0x0420
> > -#define _DPIO_DATA_LANE2 0x2620
> > -#define _DPIO_DATA_LANE3 0x2820
> > -#define DPIO_DATA_LANE_A(pipe) _PIPE(pipe, _DPIO_DATA_LANE0,
> > _DPIO_DATA_LANE2)
> > -#define DPIO_DATA_LANE_B(pipe) _PIPE(pipe, _DPIO_DATA_LANE1,
> > _DPIO_DATA_LANE3)
> > +#define DPIO_DATA_CHANNEL1 0x8220
> > +#define DPIO_DATA_CHANNEL2 0x8420
> >
> > /*
> > * Fence registers
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 68828e7..ed749c4 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4017,7 +4017,7 @@ static void vlv_update_pll(struct drm_crtc *crtc,
> >struct drm_display_mode *mode,
> >struct drm_display_mode *adjusted_mode,
> >intel_clock_t *clock, intel_clock_t *reduced_clock,
> > - int refclk, int num_connectors)
> > + int num_connectors)
> > {
> > struct drm_device *dev = crtc->dev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > @@ -4025,9 +4025,19 @@ static void vlv_update_pll(struct drm_crtc *crtc,
> > int pipe = intel_crtc->pipe;
> > u32 dpll, mdiv, pdiv;
> > u32 bestn, bestm1, bestm2, bestp1, bestp2;
> > - bool is_hdmi;
> > + bool is_sdvo;
> > + u32 temp;
> > +
> > + is_sdvo = intel_pipe_has_type(crtc, INTEL_OUTPUT_SDVO) ||
> > + intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI);
> >
> > - is_hdmi = intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI);
> > + dpll = DPLL_VGA_MODE_DIS;
> > + dpll |= DPLL_EXT_BUFFER_ENABLE_VLV;
> > + dpll |= DPLL_REFA_CLK_ENABLE_VLV;
> > + dpll |= DPLL_INTEGRATED_CLOCK_VLV;
> > +
> > + I915_WRITE(DPLL(pipe), dpll);
> > + POSTING_READ(DPLL(pipe));
> >
> > bestn = clock->n;
> > bestm1 = clock->m1;
> > @@ -4035,12 +4045,10 @@ static void vlv_update_pll(struct drm_crtc *crtc,
> > bestp1 = clock->p1;
> > bestp2 = clock->p2;
> >
> > - /* Enable DPIO clock input */
> > - dpll = DPLL_EXT_BUFFER_ENABLE_VLV | DPLL_REFA_CLK_ENABLE_VLV |
> > - DPLL_VGA_MODE_DIS | DPLL_INTEGRATED_CLOCK_VLV;
> > - I915_WRITE(DPLL(pipe), dpll);
> > - POSTING_READ(DPLL(pipe));
> > -
> > + /*
> > +* In Valleyview PLL and program lane counter registers are exposed
> > +* through DPIO interface
> > +*/
> > mdiv = ((bestm1 << DPIO_M1DIV_SHIFT) | (bestm2 & DPIO_M2DIV_MASK));
> > mdiv |= ((bestp1 << DPIO_P1_SHIFT) | (bestp2 << DPIO_P2_SHIFT));
> > mdiv |= ((bestn << DPIO_N_SHIFT));
> > @@ -4051,12 +4059,13 @@ static void vlv_update_pll(struct drm_crtc *crtc,
> >
> > intel_dpio_write(dev_priv, DPIO_CORE_CLK(pipe), 0x0100);
> >
> > - pdiv = DPIO_REFSEL_OVERRIDE | (5 << DPIO_PLL_MODESEL_SHIFT) |
> > + pdiv = (1 << DPIO_REFSEL_OVERRIDE) | (5 << DPIO_PLL_MODESEL_SHIFT) |
> > (3 << DPIO_BIAS_CURRENT_CTL_SHIFT) | (1<<20) |
> > - (8 << DPIO_DRIVER_CTL_SHIFT) | (5 << DPIO_CLK_BIAS_CTL_SHIFT);
> > + (7 << DPIO_PLL_REFCLK_SEL_SHIFT) | (8 << DPIO_DRIVER_CTL_SHIFT)
> > |
> > + (5 << DPIO_CLK_BIAS_CTL_SHIFT);
> > intel_dpio_write(dev_priv, DPIO_REFSFR(pipe), pdiv);
> >
> > - intel_dpio_write(dev_priv, DPIO_LFP_COEFF(pipe), 0x009f0051);
> > + intel_dpio_write(dev_priv, DPIO_LFP_COEFF(pipe), 0x005f003b);
> >
> > dpll |= DPLL_VCO_ENABLE;
> > I915_WRITE(DPLL(pipe), dpll);
> > @@ -4064,21 +4073,47 @@ static void vlv_update_pll(struct drm_crtc *crtc,
> > if (wait_for(((I915_READ(DPLL(pipe)) & DPLL_LOCK_VLV) ==
> > DPLL_LOCK_VLV), 1))
> > DRM_ERROR("DPLL %d fai
Re: [Intel-gfx] [PATCH v2 5/9] drm/i915: Disable CRT hotplug detection for valleyview
On Thu, Sep 27, 2012 at 08:20:15AM -0700, Jesse Barnes wrote:
> On Thu, 27 Sep 2012 19:13:05 +0530
> Vijay Purushothaman wrote:
>
> > Temporary work around to avoid spurious crt hotplug interrupts.
> >
> > Signed-off-by: Vijay Purushothaman
> > Signed-off-by: Gajanan Bhat
> > ---
> > drivers/gpu/drm/i915/intel_crt.c |7 +++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_crt.c
> > b/drivers/gpu/drm/i915/intel_crt.c
> > index c42b980..5f30364 100644
> > --- a/drivers/gpu/drm/i915/intel_crt.c
> > +++ b/drivers/gpu/drm/i915/intel_crt.c
> > @@ -308,6 +308,13 @@ static bool valleyview_crt_detect_hotplug(struct
> > drm_connector *connector)
> > bool ret;
> > u32 save_adpa;
> >
> > + /*
> > +* Disable crt detect hotplug for VLV X0. Spurious hot plug
> > +* detect calls crashses the X0 system
> > +*/
> > + if (IS_VALLEYVIEW(dev))
> > + return false;
> > +
> > save_adpa = adpa = I915_READ(ADPA);
> > DRM_DEBUG_KMS("trigger hotplug detect cycle: adpa=0x%x\n", adpa);
> >
>
> Not sure about this one; your platform seems to be less stable than
> mine when it comes to both VGA port handling and legacy VGA I/O... But
> I have no problem with it either, I just know it works ok on at least
> some of the SDVs, probably depending on board rework and firmware
> status.
>
> Acked-by: Jesse Barnes
Since it's unclear whether we need this, I'll punt on this patch here.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] This Company Continues to Show Solid Gains
Recommendation: Accummulate/Long Stock top Pick Date: Sep, 28th Name: Face Up Entertainment Group, Inc. Symbol to buy: F_UEG Today Price: $0.28 Target: $2.78 Ruling by Federal Judge That Online Poker is More of a Game of Skill than Chance. This stock is in a sweet spot!!! ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Flush outstanding unpin tasks before pageflipping
On Fri, Sep 28, 2012 at 01:07:59PM +0100, Chris Wilson wrote:
> On Fri, 28 Sep 2012 15:05:01 +0300, Ville Syrjälä
> wrote:
> > On Fri, Sep 28, 2012 at 12:29:56PM +0100, Chris Wilson wrote:
> > > If we accumulate unpin tasks because we are pageflipping faster than the
> > > system can schedule its workers, we can effectively create a
> > > pin-leak. The solution taken here is to limit the number of unpin tasks
> > > we have per-crtc and to flush those outstanding tasks if we accumulate
> > > too many. This should prevent any jitter in the normal case, and also
> > > prevent the hang if we should run too fast.
> > >
> > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=46991
> > > Signed-off-by: Chris Wilson
> > > ---
> > > drivers/gpu/drm/i915/intel_display.c | 20 +++-
> > > drivers/gpu/drm/i915/intel_drv.h |4 +++-
> > > 2 files changed, 18 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index 04407fd..14f1b51 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -6310,14 +6310,19 @@ static void intel_unpin_work_fn(struct
> > > work_struct *__work)
> > > {
> > > struct intel_unpin_work *work =
> > > container_of(__work, struct intel_unpin_work, work);
> > > + struct drm_device *dev = work->crtc->dev;
> > >
> > > - mutex_lock(&work->dev->struct_mutex);
> > > + mutex_lock(&dev->struct_mutex);
> > > intel_unpin_fb_obj(work->old_fb_obj);
> > > drm_gem_object_unreference(&work->pending_flip_obj->base);
> > > drm_gem_object_unreference(&work->old_fb_obj->base);
> > >
> > > - intel_update_fbc(work->dev);
> > > - mutex_unlock(&work->dev->struct_mutex);
> > > + intel_update_fbc(dev);
> > > + mutex_unlock(&dev->struct_mutex);
> > > +
> > > + BUG_ON(atomic_read(&to_intel_crtc(work->crtc)->unpin_work_count) == 0);
> > > + atomic_dec(&to_intel_crtc(work->crtc)->unpin_work_count);
> >
> > AFAICS you always have struct_mutex locked in the relevant functions,
> > so no need for an atomic variable.
>
> It's not in every case, since we need to do the flush without holding
> the lock, we have the choice of making this variable atomic, or taking
> and dropping the lock. Obviously I choose the former.
Ah right. I missed the conditional flush.
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Flush outstanding unpin tasks before pageflipping
On Fri, 28 Sep 2012 15:05:01 +0300, Ville Syrjälä
wrote:
> On Fri, Sep 28, 2012 at 12:29:56PM +0100, Chris Wilson wrote:
> > If we accumulate unpin tasks because we are pageflipping faster than the
> > system can schedule its workers, we can effectively create a
> > pin-leak. The solution taken here is to limit the number of unpin tasks
> > we have per-crtc and to flush those outstanding tasks if we accumulate
> > too many. This should prevent any jitter in the normal case, and also
> > prevent the hang if we should run too fast.
> >
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=46991
> > Signed-off-by: Chris Wilson
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 20 +++-
> > drivers/gpu/drm/i915/intel_drv.h |4 +++-
> > 2 files changed, 18 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 04407fd..14f1b51 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -6310,14 +6310,19 @@ static void intel_unpin_work_fn(struct work_struct
> > *__work)
> > {
> > struct intel_unpin_work *work =
> > container_of(__work, struct intel_unpin_work, work);
> > + struct drm_device *dev = work->crtc->dev;
> >
> > - mutex_lock(&work->dev->struct_mutex);
> > + mutex_lock(&dev->struct_mutex);
> > intel_unpin_fb_obj(work->old_fb_obj);
> > drm_gem_object_unreference(&work->pending_flip_obj->base);
> > drm_gem_object_unreference(&work->old_fb_obj->base);
> >
> > - intel_update_fbc(work->dev);
> > - mutex_unlock(&work->dev->struct_mutex);
> > + intel_update_fbc(dev);
> > + mutex_unlock(&dev->struct_mutex);
> > +
> > + BUG_ON(atomic_read(&to_intel_crtc(work->crtc)->unpin_work_count) == 0);
> > + atomic_dec(&to_intel_crtc(work->crtc)->unpin_work_count);
>
> AFAICS you always have struct_mutex locked in the relevant functions,
> so no need for an atomic variable.
It's not in every case, since we need to do the flush without holding
the lock, we have the choice of making this variable atomic, or taking
and dropping the lock. Obviously I choose the former.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Flush outstanding unpin tasks before pageflipping
On Fri, Sep 28, 2012 at 12:29:56PM +0100, Chris Wilson wrote:
> If we accumulate unpin tasks because we are pageflipping faster than the
> system can schedule its workers, we can effectively create a
> pin-leak. The solution taken here is to limit the number of unpin tasks
> we have per-crtc and to flush those outstanding tasks if we accumulate
> too many. This should prevent any jitter in the normal case, and also
> prevent the hang if we should run too fast.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=46991
> Signed-off-by: Chris Wilson
> ---
> drivers/gpu/drm/i915/intel_display.c | 20 +++-
> drivers/gpu/drm/i915/intel_drv.h |4 +++-
> 2 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 04407fd..14f1b51 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6310,14 +6310,19 @@ static void intel_unpin_work_fn(struct work_struct
> *__work)
> {
> struct intel_unpin_work *work =
> container_of(__work, struct intel_unpin_work, work);
> + struct drm_device *dev = work->crtc->dev;
>
> - mutex_lock(&work->dev->struct_mutex);
> + mutex_lock(&dev->struct_mutex);
> intel_unpin_fb_obj(work->old_fb_obj);
> drm_gem_object_unreference(&work->pending_flip_obj->base);
> drm_gem_object_unreference(&work->old_fb_obj->base);
>
> - intel_update_fbc(work->dev);
> - mutex_unlock(&work->dev->struct_mutex);
> + intel_update_fbc(dev);
> + mutex_unlock(&dev->struct_mutex);
> +
> + BUG_ON(atomic_read(&to_intel_crtc(work->crtc)->unpin_work_count) == 0);
> + atomic_dec(&to_intel_crtc(work->crtc)->unpin_work_count);
AFAICS you always have struct_mutex locked in the relevant functions,
so no need for an atomic variable.
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Flush the pending flips on the CRTC before modification
On Fri, 28 Sep 2012 08:37:20 +0200, Daniel Vetter wrote: > On Thu, Sep 27, 2012 at 09:25:58PM +0100, Chris Wilson wrote: > > This was meant to be the purpose of the > > intel_crtc_wait_for_pending_flips() function which is called whilst > > preparing the CRTC for a modeset or before disabling. However, as Ville > > Syrjala pointed out, we set the pending flip notification on the old > > framebuffer that is no longer attached to the CRTC by the time we come > > to flush the pending operations. Instead, we can simply wait on the > > pending unpin work to be finished on this CRTC, knowning that the > > hardware has therefore finished modifying the registers, before proceeding > > with our direct access. > > > > Signed-off-by: Chris Wilson > > Can I haz testcase plz? Fixes i-g-t/flip_test -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Flush outstanding unpin tasks before pageflipping
If we accumulate unpin tasks because we are pageflipping faster than the
system can schedule its workers, we can effectively create a
pin-leak. The solution taken here is to limit the number of unpin tasks
we have per-crtc and to flush those outstanding tasks if we accumulate
too many. This should prevent any jitter in the normal case, and also
prevent the hang if we should run too fast.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=46991
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_display.c | 20 +++-
drivers/gpu/drm/i915/intel_drv.h |4 +++-
2 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 04407fd..14f1b51 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6310,14 +6310,19 @@ static void intel_unpin_work_fn(struct work_struct
*__work)
{
struct intel_unpin_work *work =
container_of(__work, struct intel_unpin_work, work);
+ struct drm_device *dev = work->crtc->dev;
- mutex_lock(&work->dev->struct_mutex);
+ mutex_lock(&dev->struct_mutex);
intel_unpin_fb_obj(work->old_fb_obj);
drm_gem_object_unreference(&work->pending_flip_obj->base);
drm_gem_object_unreference(&work->old_fb_obj->base);
- intel_update_fbc(work->dev);
- mutex_unlock(&work->dev->struct_mutex);
+ intel_update_fbc(dev);
+ mutex_unlock(&dev->struct_mutex);
+
+ BUG_ON(atomic_read(&to_intel_crtc(work->crtc)->unpin_work_count) == 0);
+ atomic_dec(&to_intel_crtc(work->crtc)->unpin_work_count);
+
kfree(work);
}
@@ -6389,7 +6394,7 @@ static void do_intel_finish_page_flip(struct drm_device
*dev,
if (atomic_read(&obj->pending_flip) == 0)
wake_up(&dev_priv->pending_flip_queue);
- schedule_work(&work->work);
+ queue_work(dev_priv->wq, &work->work);
trace_i915_flip_complete(intel_crtc->plane, work->pending_flip_obj);
}
@@ -6690,7 +6695,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
return -ENOMEM;
work->event = event;
- work->dev = crtc->dev;
+ work->crtc = crtc;
intel_fb = to_intel_framebuffer(crtc->fb);
work->old_fb_obj = intel_fb->obj;
INIT_WORK(&work->work, intel_unpin_work_fn);
@@ -6715,6 +6720,9 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
intel_fb = to_intel_framebuffer(fb);
obj = intel_fb->obj;
+ if (atomic_read(&intel_crtc->unpin_work_count) == 2)
+ flush_workqueue(dev_priv->wq);
+
ret = i915_mutex_lock_interruptible(dev);
if (ret)
goto cleanup;
@@ -6733,6 +6741,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
* the flip occurs and the object is no longer visible.
*/
atomic_add(1 << intel_crtc->plane, &work->old_fb_obj->pending_flip);
+ atomic_inc(&intel_crtc->unpin_work_count);
ret = dev_priv->display.queue_flip(dev, crtc, fb, obj);
if (ret)
@@ -6747,6 +6756,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
return 0;
cleanup_pending:
+ atomic_dec(&intel_crtc->unpin_work_count);
atomic_sub(1 << intel_crtc->plane, &work->old_fb_obj->pending_flip);
drm_gem_object_unreference(&work->old_fb_obj->base);
drm_gem_object_unreference(&obj->base);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5515c45..acc1d08 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -203,6 +203,8 @@ struct intel_crtc {
} vblank_work;
int fdi_lanes;
+ atomic_t unpin_work_count;
+
/* Display surface base address adjustement for pageflips. Note that on
* gen4+ this only adjusts up to a tile, offsets within a tile are
* handled in the hw itself (with the TILEOFF register). */
@@ -387,7 +389,7 @@ intel_get_crtc_for_plane(struct drm_device *dev, int plane)
struct intel_unpin_work {
struct work_struct work;
- struct drm_device *dev;
+ struct drm_crtc *crtc;
struct drm_i915_gem_object *old_fb_obj;
struct drm_i915_gem_object *pending_flip_obj;
struct drm_pending_vblank_event *event;
--
1.7.10.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Call drm_handle_vblank() after processing pending pageflips
When we process a pageflip completion, we append an event to the vblank
queue. That queue is processed by drm_handle_vblank() and so by calling
drm_handle_vblank() prior to processing the pageflips, we incur an extra
frame of latency in reporting the flip event.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_irq.c | 22 ++
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 6361276..e18e56b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -564,13 +564,12 @@ static irqreturn_t valleyview_irq_handler(DRM_IRQ_ARGS)
spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
for_each_pipe(pipe) {
- if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS)
- drm_handle_vblank(dev, pipe);
-
if (pipe_stats[pipe] & PLANE_FLIPDONE_INT_STATUS_VLV) {
intel_prepare_page_flip(dev, pipe);
intel_finish_page_flip(dev, pipe);
}
+ if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS)
+ drm_handle_vblank(dev, pipe);
}
/* Consume port. Then clear IIR or we'll miss events */
@@ -2220,22 +2219,22 @@ static irqreturn_t i8xx_irq_handler(DRM_IRQ_ARGS)
if (iir & I915_USER_INTERRUPT)
notify_ring(dev, &dev_priv->ring[RCS]);
- if (pipe_stats[0] & PIPE_VBLANK_INTERRUPT_STATUS &&
- drm_handle_vblank(dev, 0)) {
+ if (pipe_stats[0] & PIPE_VBLANK_INTERRUPT_STATUS) {
if (iir & I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT) {
intel_prepare_page_flip(dev, 0);
intel_finish_page_flip(dev, 0);
flip_mask &=
~I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT;
}
+ drm_handle_vblank(dev, 0);
}
- if (pipe_stats[1] & PIPE_VBLANK_INTERRUPT_STATUS &&
- drm_handle_vblank(dev, 1)) {
+ if (pipe_stats[1] & PIPE_VBLANK_INTERRUPT_STATUS) {
if (iir & I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT) {
intel_prepare_page_flip(dev, 1);
intel_finish_page_flip(dev, 1);
flip_mask &=
~I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT;
}
+ drm_handle_vblank(dev, 1);
}
iir = new_iir;
@@ -2418,13 +2417,13 @@ static irqreturn_t i915_irq_handler(DRM_IRQ_ARGS)
int plane = pipe;
if (IS_MOBILE(dev))
plane = !plane;
- if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS &&
- drm_handle_vblank(dev, pipe)) {
+ if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS) {
if (iir & flip[plane]) {
intel_prepare_page_flip(dev, plane);
intel_finish_page_flip(dev, pipe);
flip_mask &= ~flip[plane];
}
+ drm_handle_vblank(dev, pipe);
}
if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
@@ -2662,17 +2661,16 @@ static irqreturn_t i965_irq_handler(DRM_IRQ_ARGS)
intel_prepare_page_flip(dev, 1);
for_each_pipe(pipe) {
- if (pipe_stats[pipe] &
PIPE_START_VBLANK_INTERRUPT_STATUS &&
- drm_handle_vblank(dev, pipe)) {
+ if (pipe_stats[pipe] &
PIPE_START_VBLANK_INTERRUPT_STATUS) {
i915_pageflip_stall_check(dev, pipe);
intel_finish_page_flip(dev, pipe);
+ drm_handle_vblank(dev, pipe);
}
if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
blc_event = true;
}
-
if (blc_event || (iir & I915_ASLE_INTERRUPT))
intel_opregion_asle_intr(dev);
--
1.7.10.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Flush the pending flips on the CRTC before modification
On Fri, Sep 28, 2012 at 11:22:45AM +0100, Chris Wilson wrote:
> On Fri, 28 Sep 2012 13:05:51 +0300, Ville Syrjälä
> wrote:
> > On Thu, Sep 27, 2012 at 09:25:58PM +0100, Chris Wilson wrote:
> > > This was meant to be the purpose of the
> > > intel_crtc_wait_for_pending_flips() function which is called whilst
> > > preparing the CRTC for a modeset or before disabling. However, as Ville
> > > Syrjala pointed out, we set the pending flip notification on the old
> > > framebuffer that is no longer attached to the CRTC by the time we come
> > > to flush the pending operations. Instead, we can simply wait on the
> > > pending unpin work to be finished on this CRTC, knowning that the
> > > hardware has therefore finished modifying the registers, before proceeding
> > > with our direct access.
> > >
> > > Signed-off-by: Chris Wilson
> > > ---
> > > drivers/gpu/drm/i915/intel_display.c | 24 ++--
> > > 1 file changed, 22 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index a262326..39df185 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -2896,15 +2896,36 @@ static void ironlake_fdi_disable(struct drm_crtc
> > > *crtc)
> > > udelay(100);
> > > }
> > >
> > > +static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc)
> > > +{
> > > + struct drm_device *dev = crtc->dev;
> > > + struct drm_i915_private *dev_priv = dev->dev_private;
> > > + unsigned long flags;
> > > + bool pending;
> > > +
> > > + if (atomic_read(&dev_priv->mm.wedged))
> > > + return false;
> > > +
> > > + spin_lock_irqsave(&dev->event_lock, flags);
> > > + pending = to_intel_crtc(crtc)->unpin_work != NULL;
> > > + spin_unlock_irqrestore(&dev->event_lock, flags);
> >
> > The locking looks pointless here.
>
> It does rather. Being pedagogical we should probably leave a mb of some
> sort in there...
>
> pending = to_intel_crtc(crtc)->unpin_work != NULL;
> smp_rmb();
>
> with the existing spin_lock providing the necessary barriers before the
> wake_up();
IIRC wake_up()/wait_event() already have the necessary barriers. And
based on a quick glance Documentation/memory-barriers.txt seems to
agree with me.
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Flush the pending flips on the CRTC before modification
On Fri, Sep 28, 2012 at 12:22 PM, Chris Wilson wrote:
> On Fri, 28 Sep 2012 13:05:51 +0300, Ville Syrjälä
> wrote:
>> On Thu, Sep 27, 2012 at 09:25:58PM +0100, Chris Wilson wrote:
>> > This was meant to be the purpose of the
>> > intel_crtc_wait_for_pending_flips() function which is called whilst
>> > preparing the CRTC for a modeset or before disabling. However, as Ville
>> > Syrjala pointed out, we set the pending flip notification on the old
>> > framebuffer that is no longer attached to the CRTC by the time we come
>> > to flush the pending operations. Instead, we can simply wait on the
>> > pending unpin work to be finished on this CRTC, knowning that the
>> > hardware has therefore finished modifying the registers, before proceeding
>> > with our direct access.
>> >
>> > Signed-off-by: Chris Wilson
>> > ---
>> > drivers/gpu/drm/i915/intel_display.c | 24 ++--
>> > 1 file changed, 22 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_display.c
>> > b/drivers/gpu/drm/i915/intel_display.c
>> > index a262326..39df185 100644
>> > --- a/drivers/gpu/drm/i915/intel_display.c
>> > +++ b/drivers/gpu/drm/i915/intel_display.c
>> > @@ -2896,15 +2896,36 @@ static void ironlake_fdi_disable(struct drm_crtc
>> > *crtc)
>> > udelay(100);
>> > }
>> >
>> > +static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc)
>> > +{
>> > + struct drm_device *dev = crtc->dev;
>> > + struct drm_i915_private *dev_priv = dev->dev_private;
>> > + unsigned long flags;
>> > + bool pending;
>> > +
>> > + if (atomic_read(&dev_priv->mm.wedged))
>> > + return false;
>> > +
>> > + spin_lock_irqsave(&dev->event_lock, flags);
>> > + pending = to_intel_crtc(crtc)->unpin_work != NULL;
>> > + spin_unlock_irqrestore(&dev->event_lock, flags);
>>
>> The locking looks pointless here.
>
> It does rather. Being pedagogical we should probably leave a mb of some
> sort in there...
>
> pending = to_intel_crtc(crtc)->unpin_work != NULL;
> smp_rmb();
>
> with the existing spin_lock providing the necessary barriers before the
> wake_up();
tbh I don't mind superflous spinlocks laying around ... this is
modeset code, performance at the instruction-counting level totally
does not matter. And just using spinlocks avoids the need to review
barriers ;-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Flush the pending flips on the CRTC before modification
On Fri, 28 Sep 2012 13:05:51 +0300, Ville Syrjälä
wrote:
> On Thu, Sep 27, 2012 at 09:25:58PM +0100, Chris Wilson wrote:
> > This was meant to be the purpose of the
> > intel_crtc_wait_for_pending_flips() function which is called whilst
> > preparing the CRTC for a modeset or before disabling. However, as Ville
> > Syrjala pointed out, we set the pending flip notification on the old
> > framebuffer that is no longer attached to the CRTC by the time we come
> > to flush the pending operations. Instead, we can simply wait on the
> > pending unpin work to be finished on this CRTC, knowning that the
> > hardware has therefore finished modifying the registers, before proceeding
> > with our direct access.
> >
> > Signed-off-by: Chris Wilson
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 24 ++--
> > 1 file changed, 22 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index a262326..39df185 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2896,15 +2896,36 @@ static void ironlake_fdi_disable(struct drm_crtc
> > *crtc)
> > udelay(100);
> > }
> >
> > +static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc)
> > +{
> > + struct drm_device *dev = crtc->dev;
> > + struct drm_i915_private *dev_priv = dev->dev_private;
> > + unsigned long flags;
> > + bool pending;
> > +
> > + if (atomic_read(&dev_priv->mm.wedged))
> > + return false;
> > +
> > + spin_lock_irqsave(&dev->event_lock, flags);
> > + pending = to_intel_crtc(crtc)->unpin_work != NULL;
> > + spin_unlock_irqrestore(&dev->event_lock, flags);
>
> The locking looks pointless here.
It does rather. Being pedagogical we should probably leave a mb of some
sort in there...
pending = to_intel_crtc(crtc)->unpin_work != NULL;
smp_rmb();
with the existing spin_lock providing the necessary barriers before the
wake_up();
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Flush the pending flips on the CRTC before modification
On Thu, Sep 27, 2012 at 09:25:58PM +0100, Chris Wilson wrote:
> This was meant to be the purpose of the
> intel_crtc_wait_for_pending_flips() function which is called whilst
> preparing the CRTC for a modeset or before disabling. However, as Ville
> Syrjala pointed out, we set the pending flip notification on the old
> framebuffer that is no longer attached to the CRTC by the time we come
> to flush the pending operations. Instead, we can simply wait on the
> pending unpin work to be finished on this CRTC, knowning that the
> hardware has therefore finished modifying the registers, before proceeding
> with our direct access.
>
> Signed-off-by: Chris Wilson
> ---
> drivers/gpu/drm/i915/intel_display.c | 24 ++--
> 1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index a262326..39df185 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2896,15 +2896,36 @@ static void ironlake_fdi_disable(struct drm_crtc
> *crtc)
> udelay(100);
> }
>
> +static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + unsigned long flags;
> + bool pending;
> +
> + if (atomic_read(&dev_priv->mm.wedged))
> + return false;
> +
> + spin_lock_irqsave(&dev->event_lock, flags);
> + pending = to_intel_crtc(crtc)->unpin_work != NULL;
> + spin_unlock_irqrestore(&dev->event_lock, flags);
The locking looks pointless here.
> +
> + return pending;
> +}
> +
> static void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
> {
> struct drm_device *dev = crtc->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
>
> flush_work_sync(&to_intel_crtc(crtc)->vblank_work.work);
>
> if (crtc->fb == NULL)
> return;
>
> + wait_event(dev_priv->pending_flip_queue,
> +!intel_crtc_has_pending_flip(crtc));
> +
> mutex_lock(&dev->struct_mutex);
> intel_finish_fb(crtc->fb);
> mutex_unlock(&dev->struct_mutex);
> @@ -6388,9 +6409,8 @@ static void do_intel_finish_page_flip(struct drm_device
> *dev,
>
> atomic_clear_mask(1 << intel_crtc->plane,
> &obj->pending_flip.counter);
> - if (atomic_read(&obj->pending_flip) == 0)
> - wake_up(&dev_priv->pending_flip_queue);
>
> + wake_up(&dev_priv->pending_flip_queue);
> queue_work(dev_priv->wq, &work->work);
>
> trace_i915_flip_complete(intel_crtc->plane, work->pending_flip_obj);
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Flush the pending flips on the CRTC before modification
On Fri, 28 Sep 2012 08:37:20 +0200, Daniel Vetter wrote: > On Thu, Sep 27, 2012 at 09:25:58PM +0100, Chris Wilson wrote: > > This was meant to be the purpose of the > > intel_crtc_wait_for_pending_flips() function which is called whilst > > preparing the CRTC for a modeset or before disabling. However, as Ville > > Syrjala pointed out, we set the pending flip notification on the old > > framebuffer that is no longer attached to the CRTC by the time we come > > to flush the pending operations. Instead, we can simply wait on the > > pending unpin work to be finished on this CRTC, knowning that the > > hardware has therefore finished modifying the registers, before proceeding > > with our direct access. > > > > Signed-off-by: Chris Wilson > > Can I haz testcase plz? Compiz? :-p -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Flush the pending flips on the CRTC before modification
On Thu, Sep 27, 2012 at 09:25:58PM +0100, Chris Wilson wrote:
> This was meant to be the purpose of the
> intel_crtc_wait_for_pending_flips() function which is called whilst
> preparing the CRTC for a modeset or before disabling. However, as Ville
> Syrjala pointed out, we set the pending flip notification on the old
> framebuffer that is no longer attached to the CRTC by the time we come
> to flush the pending operations. Instead, we can simply wait on the
> pending unpin work to be finished on this CRTC, knowning that the
> hardware has therefore finished modifying the registers, before proceeding
> with our direct access.
>
> Signed-off-by: Chris Wilson
Can I haz testcase plz?
Shouldn't be too hard to race a few pageflips with dpms off and crtc
disabling, maybe we need to add some busy load onto the gpu first to delay
things for long enough.
Thanks, Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c | 24 ++--
> 1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index a262326..39df185 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2896,15 +2896,36 @@ static void ironlake_fdi_disable(struct drm_crtc
> *crtc)
> udelay(100);
> }
>
> +static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + unsigned long flags;
> + bool pending;
> +
> + if (atomic_read(&dev_priv->mm.wedged))
> + return false;
> +
> + spin_lock_irqsave(&dev->event_lock, flags);
> + pending = to_intel_crtc(crtc)->unpin_work != NULL;
> + spin_unlock_irqrestore(&dev->event_lock, flags);
> +
> + return pending;
> +}
> +
> static void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
> {
> struct drm_device *dev = crtc->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
>
> flush_work_sync(&to_intel_crtc(crtc)->vblank_work.work);
>
> if (crtc->fb == NULL)
> return;
>
> + wait_event(dev_priv->pending_flip_queue,
> +!intel_crtc_has_pending_flip(crtc));
> +
> mutex_lock(&dev->struct_mutex);
> intel_finish_fb(crtc->fb);
> mutex_unlock(&dev->struct_mutex);
> @@ -6388,9 +6409,8 @@ static void do_intel_finish_page_flip(struct drm_device
> *dev,
>
> atomic_clear_mask(1 << intel_crtc->plane,
> &obj->pending_flip.counter);
> - if (atomic_read(&obj->pending_flip) == 0)
> - wake_up(&dev_priv->pending_flip_queue);
>
> + wake_up(&dev_priv->pending_flip_queue);
> queue_work(dev_priv->wq, &work->work);
>
> trace_i915_flip_complete(intel_crtc->plane, work->pending_flip_obj);
> --
> 1.7.10.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] This company is making noise
Advice: Purchase/Stay This stock is cheap Date: Friday, Sep 28th Name: FaceUp Entertainment Group Inc Ticker: F_U E_G Closed yesterday: $.28 Target Price: $1.47 Ruling of Federal Judge Weinstein That Internet poker is a game of Skill. This stock is in a sweet spot. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
