* [PATCH 2/6] drm/i915: use compute_config in set_config
2014-10-23 18:50 [PATCH 1/6] drm/i915: factor out compute_config from __intel_set_mode Jesse Barnes
@ 2014-10-23 18:50 ` Jesse Barnes
2014-10-29 14:31 ` Ander Conselvan de Oliveira
2014-10-23 18:50 ` [PATCH 3/6] drm/i915/hdmi: fetch infoframe status in get_config Jesse Barnes
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Jesse Barnes @ 2014-10-23 18:50 UTC (permalink / raw)
To: intel-gfx
This will allow us to consult more info before deciding whether to flip
or do a full mode set.
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
drivers/gpu/drm/i915/intel_display.c | 36 ++++++++++++++++++++++++++++++------
1 file changed, 30 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d5f95e4..e031ee8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11427,6 +11427,7 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
struct drm_device *dev;
struct drm_mode_set save_set;
struct intel_set_config *config;
+ unsigned modeset_pipes, prepare_pipes, disable_pipes;
int ret;
BUG_ON(!set);
@@ -11472,9 +11473,17 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
if (ret)
goto fail;
+ ret = intel_modeset_compute_config(set->crtc, set->mode, set->fb,
+ &modeset_pipes, &prepare_pipes,
+ &disable_pipes);
+ if (ret)
+ goto fail;
+
if (config->mode_changed) {
- ret = intel_set_mode(set->crtc, set->mode,
- set->x, set->y, set->fb);
+ ret = intel_set_mode_pipes(set->crtc, set->mode,
+ set->x, set->y, set->fb,
+ modeset_pipes, prepare_pipes,
+ disable_pipes);
} else if (config->fb_changed) {
struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
@@ -11521,8 +11530,10 @@ fail:
/* Try to restore the config */
if (config->mode_changed &&
- intel_set_mode(save_set.crtc, save_set.mode,
- save_set.x, save_set.y, save_set.fb))
+ intel_set_mode_pipes(save_set.crtc, save_set.mode,
+ save_set.x, save_set.y, save_set.fb,
+ modeset_pipes, prepare_pipes,
+ disable_pipes))
DRM_ERROR("failed to restore config after modeset failure\n");
}
@@ -13409,14 +13420,27 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
for_each_pipe(dev_priv, pipe) {
struct drm_crtc *crtc =
dev_priv->pipe_to_crtc_mapping[pipe];
+ unsigned modeset_pipes, prepare_pipes, disable_pipes;
+ int ret;
+
+ ret = intel_modeset_compute_config(crtc, &crtc->mode,
+ crtc->primary->fb,
+ &modeset_pipes,
+ &prepare_pipes,
+ &disable_pipes);
+ if (ret) {
+ DRM_DEBUG_KMS("prepare failed\n");
+ goto out;
+ }
__intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y,
- crtc->primary->fb);
+ crtc->primary->fb, modeset_pipes,
+ prepare_pipes, disable_pipes);
}
} else {
intel_modeset_update_staged_output_state(dev);
}
-
+out:
intel_modeset_check_state(dev);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 2/6] drm/i915: use compute_config in set_config
2014-10-23 18:50 ` [PATCH 2/6] drm/i915: use compute_config in set_config Jesse Barnes
@ 2014-10-29 14:31 ` Ander Conselvan de Oliveira
2014-10-30 18:47 ` Jesse Barnes
0 siblings, 1 reply; 14+ messages in thread
From: Ander Conselvan de Oliveira @ 2014-10-29 14:31 UTC (permalink / raw)
To: Jesse Barnes, intel-gfx
On 10/23/2014 09:50 PM, Jesse Barnes wrote:
> This will allow us to consult more info before deciding whether to flip
> or do a full mode set.
>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
> drivers/gpu/drm/i915/intel_display.c | 36 ++++++++++++++++++++++++++++++------
> 1 file changed, 30 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d5f95e4..e031ee8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11427,6 +11427,7 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
> struct drm_device *dev;
> struct drm_mode_set save_set;
> struct intel_set_config *config;
> + unsigned modeset_pipes, prepare_pipes, disable_pipes;
> int ret;
>
> BUG_ON(!set);
> @@ -11472,9 +11473,17 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
> if (ret)
> goto fail;
If this fails ...
>
> + ret = intel_modeset_compute_config(set->crtc, set->mode, set->fb,
> + &modeset_pipes, &prepare_pipes,
> + &disable_pipes);
> + if (ret)
> + goto fail;
> +
> if (config->mode_changed) {
> - ret = intel_set_mode(set->crtc, set->mode,
> - set->x, set->y, set->fb);
> + ret = intel_set_mode_pipes(set->crtc, set->mode,
> + set->x, set->y, set->fb,
> + modeset_pipes, prepare_pipes,
> + disable_pipes);
> } else if (config->fb_changed) {
> struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
>
> @@ -11521,8 +11530,10 @@ fail:
>
> /* Try to restore the config */
> if (config->mode_changed &&
> - intel_set_mode(save_set.crtc, save_set.mode,
> - save_set.x, save_set.y, save_set.fb))
> + intel_set_mode_pipes(save_set.crtc, save_set.mode,
> + save_set.x, save_set.y, save_set.fb,
> + modeset_pipes, prepare_pipes,
> + disable_pipes))
... we end up here, with *_pipes uninitialized. And in any case, they
were computed for the new mode, not the saved one.
Ander
> DRM_ERROR("failed to restore config after modeset failure\n");
> }
>
> @@ -13409,14 +13420,27 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
> for_each_pipe(dev_priv, pipe) {
> struct drm_crtc *crtc =
> dev_priv->pipe_to_crtc_mapping[pipe];
> + unsigned modeset_pipes, prepare_pipes, disable_pipes;
> + int ret;
> +
> + ret = intel_modeset_compute_config(crtc, &crtc->mode,
> + crtc->primary->fb,
> + &modeset_pipes,
> + &prepare_pipes,
> + &disable_pipes);
> + if (ret) {
> + DRM_DEBUG_KMS("prepare failed\n");
> + goto out;
> + }
>
> __intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y,
> - crtc->primary->fb);
> + crtc->primary->fb, modeset_pipes,
> + prepare_pipes, disable_pipes);
> }
> } else {
> intel_modeset_update_staged_output_state(dev);
> }
> -
> +out:
> intel_modeset_check_state(dev);
> }
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 2/6] drm/i915: use compute_config in set_config
2014-10-29 14:31 ` Ander Conselvan de Oliveira
@ 2014-10-30 18:47 ` Jesse Barnes
0 siblings, 0 replies; 14+ messages in thread
From: Jesse Barnes @ 2014-10-30 18:47 UTC (permalink / raw)
To: Ander Conselvan de Oliveira; +Cc: intel-gfx
On Wed, 29 Oct 2014 16:31:56 +0200
Ander Conselvan de Oliveira <conselvan2@gmail.com> wrote:
> On 10/23/2014 09:50 PM, Jesse Barnes wrote:
> > This will allow us to consult more info before deciding whether to
> > flip or do a full mode set.
> >
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 36
> > ++++++++++++++++++++++++++++++------ 1 file changed, 30
> > insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c index d5f95e4..e031ee8 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -11427,6 +11427,7 @@ static int intel_crtc_set_config(struct
> > drm_mode_set *set) struct drm_device *dev;
> > struct drm_mode_set save_set;
> > struct intel_set_config *config;
> > + unsigned modeset_pipes, prepare_pipes, disable_pipes;
> > int ret;
> >
> > BUG_ON(!set);
> > @@ -11472,9 +11473,17 @@ static int intel_crtc_set_config(struct
> > drm_mode_set *set) if (ret)
> > goto fail;
>
> If this fails ...
>
> >
> > + ret = intel_modeset_compute_config(set->crtc, set->mode,
> > set->fb,
> > + &modeset_pipes,
> > &prepare_pipes,
> > + &disable_pipes);
> > + if (ret)
> > + goto fail;
> > +
> > if (config->mode_changed) {
> > - ret = intel_set_mode(set->crtc, set->mode,
> > - set->x, set->y, set->fb);
> > + ret = intel_set_mode_pipes(set->crtc, set->mode,
> > + set->x, set->y, set->fb,
> > + modeset_pipes,
> > prepare_pipes,
> > + disable_pipes);
> > } else if (config->fb_changed) {
> > struct intel_crtc *intel_crtc =
> > to_intel_crtc(set->crtc);
> >
> > @@ -11521,8 +11530,10 @@ fail:
> >
> > /* Try to restore the config */
> > if (config->mode_changed &&
> > - intel_set_mode(save_set.crtc, save_set.mode,
> > - save_set.x, save_set.y,
> > save_set.fb))
> > + intel_set_mode_pipes(save_set.crtc,
> > save_set.mode,
> > + save_set.x, save_set.y,
> > save_set.fb,
> > + modeset_pipes,
> > prepare_pipes,
> > + disable_pipes))
>
> ... we end up here, with *_pipes uninitialized. And in any case, they
> were computed for the new mode, not the saved one.
Yeah I should just be using the old intel_set_mode() here on the
failure path. That should calculate a new set of pipes and the
appropriate config and apply it. Fixed.
Thanks,
Jesse
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/6] drm/i915/hdmi: fetch infoframe status in get_config
2014-10-23 18:50 [PATCH 1/6] drm/i915: factor out compute_config from __intel_set_mode Jesse Barnes
2014-10-23 18:50 ` [PATCH 2/6] drm/i915: use compute_config in set_config Jesse Barnes
@ 2014-10-23 18:50 ` Jesse Barnes
2014-10-30 13:20 ` Ander Conselvan de Oliveira
2014-10-23 18:50 ` [PATCH 4/6] drm/i915: check for audio and infoframe changes across mode sets Jesse Barnes
` (3 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Jesse Barnes @ 2014-10-23 18:50 UTC (permalink / raw)
To: intel-gfx
This is useful for checking things later.
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
drivers/gpu/drm/i915/intel_drv.h | 4 +++
drivers/gpu/drm/i915/intel_hdmi.c | 61 +++++++++++++++++++++++++++++++++++++++
2 files changed, 65 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5ab813c..cdace63 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -290,6 +290,9 @@ struct intel_crtc_config {
* between pch encoders and cpu encoders. */
bool has_pch_encoder;
+ /* Are we sending infoframes on the attached port */
+ bool has_infoframe;
+
/* CPU Transcoder for the pipe. Currently this can only differ from the
* pipe on Haswell (where we have a special eDP transcoder). */
enum transcoder cpu_transcoder;
@@ -541,6 +544,7 @@ struct intel_hdmi {
void (*set_infoframes)(struct drm_encoder *encoder,
bool enable,
struct drm_display_mode *adjusted_mode);
+ bool (*infoframe_enabled)(struct drm_encoder *encoder);
};
struct intel_dp_mst_encoder;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 8b5f3aa..75efe4c 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -166,6 +166,15 @@ static void g4x_write_infoframe(struct drm_encoder *encoder,
POSTING_READ(VIDEO_DIP_CTL);
}
+static bool g4x_infoframe_enabled(struct drm_encoder *encoder)
+{
+ struct drm_device *dev = encoder->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ u32 val = I915_READ(VIDEO_DIP_CTL);
+
+ return val & VIDEO_DIP_ENABLE;
+}
+
static void ibx_write_infoframe(struct drm_encoder *encoder,
enum hdmi_infoframe_type type,
const void *frame, ssize_t len)
@@ -204,6 +213,17 @@ static void ibx_write_infoframe(struct drm_encoder *encoder,
POSTING_READ(reg);
}
+static bool ibx_infoframe_enabled(struct drm_encoder *encoder)
+{
+ struct drm_device *dev = encoder->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+ int reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
+ u32 val = I915_READ(reg);
+
+ return val & VIDEO_DIP_ENABLE;
+}
+
static void cpt_write_infoframe(struct drm_encoder *encoder,
enum hdmi_infoframe_type type,
const void *frame, ssize_t len)
@@ -245,6 +265,17 @@ static void cpt_write_infoframe(struct drm_encoder *encoder,
POSTING_READ(reg);
}
+static bool cpt_infoframe_enabled(struct drm_encoder *encoder)
+{
+ struct drm_device *dev = encoder->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+ int reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
+ u32 val = I915_READ(reg);
+
+ return val & VIDEO_DIP_ENABLE;
+}
+
static void vlv_write_infoframe(struct drm_encoder *encoder,
enum hdmi_infoframe_type type,
const void *frame, ssize_t len)
@@ -283,6 +314,17 @@ static void vlv_write_infoframe(struct drm_encoder *encoder,
POSTING_READ(reg);
}
+static bool vlv_infoframe_enabled(struct drm_encoder *encoder)
+{
+ struct drm_device *dev = encoder->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+ int reg = VLV_TVIDEO_DIP_CTL(intel_crtc->pipe);
+ u32 val = I915_READ(reg);
+
+ return val & VIDEO_DIP_ENABLE;
+}
+
static void hsw_write_infoframe(struct drm_encoder *encoder,
enum hdmi_infoframe_type type,
const void *frame, ssize_t len)
@@ -320,6 +362,17 @@ static void hsw_write_infoframe(struct drm_encoder *encoder,
POSTING_READ(ctl_reg);
}
+static bool hsw_infoframe_enabled(struct drm_encoder *encoder)
+{
+ struct drm_device *dev = encoder->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+ u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->config.cpu_transcoder);
+ u32 val = I915_READ(ctl_reg);
+
+ return val & VIDEO_DIP_ENABLE;
+}
+
/*
* The data we write to the DIP data buffer registers is 1 byte bigger than the
* HDMI infoframe size because of an ECC/reserved byte at position 3 (starting
@@ -732,6 +785,9 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
if (tmp & HDMI_MODE_SELECT_HDMI)
pipe_config->has_hdmi_sink = true;
+ if (intel_hdmi->infoframe_enabled(&encoder->base))
+ pipe_config->has_infoframe = true;
+
if (tmp & SDVO_AUDIO_ENABLE)
pipe_config->has_audio = true;
@@ -1616,18 +1672,23 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
if (IS_VALLEYVIEW(dev)) {
intel_hdmi->write_infoframe = vlv_write_infoframe;
intel_hdmi->set_infoframes = vlv_set_infoframes;
+ intel_hdmi->infoframe_enabled = vlv_infoframe_enabled;
} else if (IS_G4X(dev)) {
intel_hdmi->write_infoframe = g4x_write_infoframe;
intel_hdmi->set_infoframes = g4x_set_infoframes;
+ intel_hdmi->infoframe_enabled = g4x_infoframe_enabled;
} else if (HAS_DDI(dev)) {
intel_hdmi->write_infoframe = hsw_write_infoframe;
intel_hdmi->set_infoframes = hsw_set_infoframes;
+ intel_hdmi->infoframe_enabled = hsw_infoframe_enabled;
} else if (HAS_PCH_IBX(dev)) {
intel_hdmi->write_infoframe = ibx_write_infoframe;
intel_hdmi->set_infoframes = ibx_set_infoframes;
+ intel_hdmi->infoframe_enabled = ibx_infoframe_enabled;
} else {
intel_hdmi->write_infoframe = cpt_write_infoframe;
intel_hdmi->set_infoframes = cpt_set_infoframes;
+ intel_hdmi->infoframe_enabled = cpt_infoframe_enabled;
}
if (HAS_DDI(dev))
--
1.9.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 3/6] drm/i915/hdmi: fetch infoframe status in get_config
2014-10-23 18:50 ` [PATCH 3/6] drm/i915/hdmi: fetch infoframe status in get_config Jesse Barnes
@ 2014-10-30 13:20 ` Ander Conselvan de Oliveira
2014-10-30 18:49 ` Jesse Barnes
0 siblings, 1 reply; 14+ messages in thread
From: Ander Conselvan de Oliveira @ 2014-10-30 13:20 UTC (permalink / raw)
To: Jesse Barnes, intel-gfx
On 10/23/2014 09:50 PM, Jesse Barnes wrote:
> This is useful for checking things later.
>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
> drivers/gpu/drm/i915/intel_drv.h | 4 +++
> drivers/gpu/drm/i915/intel_hdmi.c | 61 +++++++++++++++++++++++++++++++++++++++
> 2 files changed, 65 insertions(+)
>
[...]
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 8b5f3aa..75efe4c 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
[...]
> @@ -320,6 +362,17 @@ static void hsw_write_infoframe(struct drm_encoder *encoder,
> POSTING_READ(ctl_reg);
> }
>
> +static bool hsw_infoframe_enabled(struct drm_encoder *encoder)
> +{
> + struct drm_device *dev = encoder->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
> + u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->config.cpu_transcoder);
> + u32 val = I915_READ(ctl_reg);
> +
> + return val & VIDEO_DIP_ENABLE;
Haswell docs list bit 31 of VIDEO_DIP_CTL as reserved. Looking at
hsw_set_infoframe(), it seems that you would have to test for the enable
bits for the different infoframe types instead.
Ander
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 3/6] drm/i915/hdmi: fetch infoframe status in get_config
2014-10-30 13:20 ` Ander Conselvan de Oliveira
@ 2014-10-30 18:49 ` Jesse Barnes
0 siblings, 0 replies; 14+ messages in thread
From: Jesse Barnes @ 2014-10-30 18:49 UTC (permalink / raw)
To: Ander Conselvan de Oliveira; +Cc: intel-gfx
On Thu, 30 Oct 2014 15:20:43 +0200
Ander Conselvan de Oliveira <conselvan2@gmail.com> wrote:
> On 10/23/2014 09:50 PM, Jesse Barnes wrote:
> > This is useful for checking things later.
> >
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> > drivers/gpu/drm/i915/intel_drv.h | 4 +++
> > drivers/gpu/drm/i915/intel_hdmi.c | 61
> > +++++++++++++++++++++++++++++++++++++++ 2 files changed, 65
> > insertions(+)
> >
>
> [...]
>
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> > b/drivers/gpu/drm/i915/intel_hdmi.c index 8b5f3aa..75efe4c 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>
> [...]
>
> > @@ -320,6 +362,17 @@ static void hsw_write_infoframe(struct
> > drm_encoder *encoder, POSTING_READ(ctl_reg);
> > }
> >
> > +static bool hsw_infoframe_enabled(struct drm_encoder *encoder)
> > +{
> > + struct drm_device *dev = encoder->dev;
> > + struct drm_i915_private *dev_priv = dev->dev_private;
> > + struct intel_crtc *intel_crtc =
> > to_intel_crtc(encoder->crtc);
> > + u32 ctl_reg =
> > HSW_TVIDEO_DIP_CTL(intel_crtc->config.cpu_transcoder);
> > + u32 val = I915_READ(ctl_reg);
> > +
> > + return val & VIDEO_DIP_ENABLE;
>
> Haswell docs list bit 31 of VIDEO_DIP_CTL as reserved. Looking at
> hsw_set_infoframe(), it seems that you would have to test for the
> enable bits for the different infoframe types instead.
Ah right, fixed.
Thanks,
Jesse
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 4/6] drm/i915: check for audio and infoframe changes across mode sets
2014-10-23 18:50 [PATCH 1/6] drm/i915: factor out compute_config from __intel_set_mode Jesse Barnes
2014-10-23 18:50 ` [PATCH 2/6] drm/i915: use compute_config in set_config Jesse Barnes
2014-10-23 18:50 ` [PATCH 3/6] drm/i915/hdmi: fetch infoframe status in get_config Jesse Barnes
@ 2014-10-23 18:50 ` Jesse Barnes
2014-10-23 18:50 ` [PATCH 5/6] drm/i915: update pipe size at set_config time Jesse Barnes
` (2 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Jesse Barnes @ 2014-10-23 18:50 UTC (permalink / raw)
To: intel-gfx
If these change (e.g. after a modeset following a fastboot), we need to
do a full mode set.
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
drivers/gpu/drm/i915/intel_display.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e031ee8..235933a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11479,6 +11479,16 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
if (ret)
goto fail;
+ if (to_intel_crtc(set->crtc)->new_config->has_audio !=
+ to_intel_crtc(set->crtc)->config.has_audio)
+ config->mode_changed = true;
+
+ /* Force mode sets for any infoframe stuff */
+ if (to_intel_crtc(set->crtc)->new_config->has_infoframe ||
+ to_intel_crtc(set->crtc)->config.has_infoframe)
+ config->mode_changed = true;
+
+
if (config->mode_changed) {
ret = intel_set_mode_pipes(set->crtc, set->mode,
set->x, set->y, set->fb,
--
1.9.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 5/6] drm/i915: update pipe size at set_config time
2014-10-23 18:50 [PATCH 1/6] drm/i915: factor out compute_config from __intel_set_mode Jesse Barnes
` (2 preceding siblings ...)
2014-10-23 18:50 ` [PATCH 4/6] drm/i915: check for audio and infoframe changes across mode sets Jesse Barnes
@ 2014-10-23 18:50 ` Jesse Barnes
2014-10-23 18:50 ` [PATCH 6/6] drm/i915: calculate pfit changes in set_config Jesse Barnes
2014-10-29 14:30 ` [PATCH 1/6] drm/i915: factor out compute_config from __intel_set_mode Ander Conselvan de Oliveira
5 siblings, 0 replies; 14+ messages in thread
From: Jesse Barnes @ 2014-10-23 18:50 UTC (permalink / raw)
To: intel-gfx
This only affects the fastboot path as-is. In that case, we simply need
to make sure that we update the pipe size at the first mode set. Rather
than putting it off until we decide to flip (if indeed we do end up
flipping), update the pipe size as appropriate a bit earlier in the
set_config call.
This sets us up for better pipe tracking in later patches.
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
drivers/gpu/drm/i915/intel_display.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 235933a..10468a7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2902,8 +2902,6 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
return ret;
}
- intel_update_pipe_size(intel_crtc);
-
dev_priv->display.update_primary_plane(crtc, fb, x, y);
if (intel_crtc->active)
@@ -11488,6 +11486,7 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
to_intel_crtc(set->crtc)->config.has_infoframe)
config->mode_changed = true;
+ intel_update_pipe_size(to_intel_crtc(set->crtc));
if (config->mode_changed) {
ret = intel_set_mode_pipes(set->crtc, set->mode,
--
1.9.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 6/6] drm/i915: calculate pfit changes in set_config
2014-10-23 18:50 [PATCH 1/6] drm/i915: factor out compute_config from __intel_set_mode Jesse Barnes
` (3 preceding siblings ...)
2014-10-23 18:50 ` [PATCH 5/6] drm/i915: update pipe size at set_config time Jesse Barnes
@ 2014-10-23 18:50 ` Jesse Barnes
2014-10-30 15:40 ` Ander Conselvan de Oliveira
2014-10-29 14:30 ` [PATCH 1/6] drm/i915: factor out compute_config from __intel_set_mode Ander Conselvan de Oliveira
5 siblings, 1 reply; 14+ messages in thread
From: Jesse Barnes @ 2014-10-23 18:50 UTC (permalink / raw)
To: intel-gfx
This should allow us to avoid mode sets for some panel fitter config
changes.
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
drivers/gpu/drm/i915/intel_display.c | 48 ++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 10468a7..84331a3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11420,6 +11420,50 @@ static void disable_crtc_nofb(struct intel_crtc *crtc)
crtc->new_config = NULL;
}
+/* Do we need a mode set due to pfit changes? */
+static bool intel_pfit_changed(struct drm_device *dev,
+ struct intel_crtc_config *new_config,
+ struct intel_crtc_config *cur_config)
+{
+ bool ret = false;
+
+ if (HAS_DDI(dev) || HAS_PCH_SPLIT(dev)) {
+ /*
+ * On PCH platforms we can disable pfit w/o a pipe shutdown,
+ * otherwise we'll need a mode set.
+ */
+ if (new_config->pch_pfit.enabled &&
+ cur_config->pch_pfit.enabled)
+ ret = false;
+ else if (new_config->pch_pfit.enabled &&
+ !cur_config->pch_pfit.enabled)
+ ret = true;
+ else if (!new_config->pch_pfit.enabled &&
+ cur_config->pch_pfit.enabled)
+ ret = false;
+ else if (!new_config->pch_pfit.enabled &&
+ !cur_config->pch_pfit.enabled)
+ ret = false;
+ } else {
+ bool new_enabled, old_enabled;
+
+ new_enabled = !!(new_config->gmch_pfit.control & PFIT_ENABLE);
+ old_enabled = !!(cur_config->gmch_pfit.control & PFIT_ENABLE);
+
+ /* 9xx only needs a shutdown to disable pfit */
+ if (new_enabled && old_enabled)
+ ret = false;
+ else if (new_enabled && !old_enabled)
+ ret = false;
+ else if (!new_enabled && old_enabled)
+ ret = true;
+ else if (!new_enabled && !old_enabled)
+ ret = false;
+ }
+
+ return ret;
+}
+
static int intel_crtc_set_config(struct drm_mode_set *set)
{
struct drm_device *dev;
@@ -11486,6 +11530,10 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
to_intel_crtc(set->crtc)->config.has_infoframe)
config->mode_changed = true;
+ if (intel_pfit_changed(dev, to_intel_crtc(set->crtc)->new_config,
+ &to_intel_crtc(set->crtc)->config))
+ config->mode_changed = true;
+
intel_update_pipe_size(to_intel_crtc(set->crtc));
if (config->mode_changed) {
--
1.9.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 6/6] drm/i915: calculate pfit changes in set_config
2014-10-23 18:50 ` [PATCH 6/6] drm/i915: calculate pfit changes in set_config Jesse Barnes
@ 2014-10-30 15:40 ` Ander Conselvan de Oliveira
0 siblings, 0 replies; 14+ messages in thread
From: Ander Conselvan de Oliveira @ 2014-10-30 15:40 UTC (permalink / raw)
To: Jesse Barnes, intel-gfx
On 10/23/2014 09:50 PM, Jesse Barnes wrote:
> This should allow us to avoid mode sets for some panel fitter config
> changes.
>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
> drivers/gpu/drm/i915/intel_display.c | 48 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 48 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 10468a7..84331a3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11420,6 +11420,50 @@ static void disable_crtc_nofb(struct intel_crtc *crtc)
> crtc->new_config = NULL;
> }
>
> +/* Do we need a mode set due to pfit changes? */
> +static bool intel_pfit_changed(struct drm_device *dev,
> + struct intel_crtc_config *new_config,
> + struct intel_crtc_config *cur_config)
> +{
> + bool ret = false;
> +
> + if (HAS_DDI(dev) || HAS_PCH_SPLIT(dev)) {
> + /*
> + * On PCH platforms we can disable pfit w/o a pipe shutdown,
> + * otherwise we'll need a mode set.
> + */
> + if (new_config->pch_pfit.enabled &&
> + cur_config->pch_pfit.enabled)
> + ret = false;
> + else if (new_config->pch_pfit.enabled &&
> + !cur_config->pch_pfit.enabled)
> + ret = true;
> + else if (!new_config->pch_pfit.enabled &&
> + cur_config->pch_pfit.enabled)
> + ret = false;
> + else if (!new_config->pch_pfit.enabled &&
> + !cur_config->pch_pfit.enabled)
> + ret = false;
> + } else {
> + bool new_enabled, old_enabled;
> +
> + new_enabled = !!(new_config->gmch_pfit.control & PFIT_ENABLE);
> + old_enabled = !!(cur_config->gmch_pfit.control & PFIT_ENABLE);
> +
> + /* 9xx only needs a shutdown to disable pfit */
> + if (new_enabled && old_enabled)
> + ret = false;
> + else if (new_enabled && !old_enabled)
> + ret = false;
> + else if (!new_enabled && old_enabled)
> + ret = true;
> + else if (!new_enabled && !old_enabled)
> + ret = false;
> + }
> +
> + return ret;
> +}
> +
> static int intel_crtc_set_config(struct drm_mode_set *set)
> {
> struct drm_device *dev;
> @@ -11486,6 +11530,10 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
> to_intel_crtc(set->crtc)->config.has_infoframe)
> config->mode_changed = true;
>
> + if (intel_pfit_changed(dev, to_intel_crtc(set->crtc)->new_config,
> + &to_intel_crtc(set->crtc)->config))
> + config->mode_changed = true;
> +
> intel_update_pipe_size(to_intel_crtc(set->crtc));
The big comment in intel_update_pipe_size() becomes obsolete with this
patch. It should probably just be removed.
With the comments I made addressed, this series is
Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/6] drm/i915: factor out compute_config from __intel_set_mode
2014-10-23 18:50 [PATCH 1/6] drm/i915: factor out compute_config from __intel_set_mode Jesse Barnes
` (4 preceding siblings ...)
2014-10-23 18:50 ` [PATCH 6/6] drm/i915: calculate pfit changes in set_config Jesse Barnes
@ 2014-10-29 14:30 ` Ander Conselvan de Oliveira
2014-10-30 18:38 ` Jesse Barnes
5 siblings, 1 reply; 14+ messages in thread
From: Ander Conselvan de Oliveira @ 2014-10-29 14:30 UTC (permalink / raw)
To: Jesse Barnes, intel-gfx
On 10/23/2014 09:50 PM, Jesse Barnes wrote:
> This allows us to calculate the full pipe config before we do any mode
> setting work.
>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
> drivers/gpu/drm/i915/intel_display.c | 93 +++++++++++++++++++++++++-----------
> 1 file changed, 65 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 6e5bc3c..d5f95e4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10910,45 +10910,62 @@ static void update_scanline_offset(struct intel_crtc *crtc)
> crtc->scanline_offset = 1;
> }
>
> +static int intel_modeset_compute_config(struct drm_crtc *crtc,
s/drm_crtc/intel_crtc/
> + struct drm_display_mode *mode,
> + struct drm_framebuffer *fb,
> + unsigned *modeset_pipes,
> + unsigned *prepare_pipes,
> + unsigned *disable_pipes)
> +{
> + struct intel_crtc_config *pipe_config = NULL;
> + int ret = 0;
> +
> + intel_modeset_affected_pipes(crtc, modeset_pipes,
> + prepare_pipes, disable_pipes);
> +
> + if (!modeset_pipes)
> + goto out;
> +
> + /* Hack: Because we don't (yet) support global modeset on multiple
> + * crtcs, we don't keep track of the new mode for more than one crtc.
> + * Hence simply check whether any bit is set in modeset_pipes in all the
> + * pieces of code that are not yet converted to deal with mutliple crtcs
> + * changing their mode at the same time. */
This comment seems out of place here since it refers to checking for set
bits in modeset_pipes. I guess the important part of it is to explain
the fact that only one new pipe config is obtained here, so maybe it can
reworded to make that clearer.
And there's another "if (modeset_pipes)" left in __intel_set_mode() to
which this comment applies.
> + pipe_config = intel_modeset_pipe_config(crtc, fb, mode);
> + if (IS_ERR(pipe_config)) {
> + ret = PTR_ERR(pipe_config);
> + pipe_config = NULL;
> +
> + goto out;
Nothing will use pipe_config after this point, so there's no need to set
it to NULL.
> + }
> + intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
> + "[modeset]");
> + to_intel_crtc(crtc)->new_config = pipe_config;
> +
> +out:
> + return ret;
There's nothing being done here, so we could avoid this goto dance and
just use returns instead.
Anyway, mostly nit picks, so either way,
Reviewed-by: Ander Conselvan de Oliveira
<ander.conselvan.de.oliveira@intel.com>
> +}
> +
> static int __intel_set_mode(struct drm_crtc *crtc,
> struct drm_display_mode *mode,
> - int x, int y, struct drm_framebuffer *fb)
> + int x, int y, struct drm_framebuffer *fb,
> + unsigned disable_pipes,
> + unsigned prepare_pipes,
> + unsigned modeset_pipes)
> {
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct drm_display_mode *saved_mode;
> struct intel_crtc_config *pipe_config = NULL;
> struct intel_crtc *intel_crtc;
> - unsigned disable_pipes, prepare_pipes, modeset_pipes;
> int ret = 0;
>
> saved_mode = kmalloc(sizeof(*saved_mode), GFP_KERNEL);
> if (!saved_mode)
> return -ENOMEM;
>
> - intel_modeset_affected_pipes(crtc, &modeset_pipes,
> - &prepare_pipes, &disable_pipes);
> -
> *saved_mode = crtc->mode;
>
> - /* Hack: Because we don't (yet) support global modeset on multiple
> - * crtcs, we don't keep track of the new mode for more than one crtc.
> - * Hence simply check whether any bit is set in modeset_pipes in all the
> - * pieces of code that are not yet converted to deal with mutliple crtcs
> - * changing their mode at the same time. */
> - if (modeset_pipes) {
> - pipe_config = intel_modeset_pipe_config(crtc, fb, mode);
> - if (IS_ERR(pipe_config)) {
> - ret = PTR_ERR(pipe_config);
> - pipe_config = NULL;
> -
> - goto out;
> - }
> - intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
> - "[modeset]");
> - to_intel_crtc(crtc)->new_config = pipe_config;
> - }
> -
> /*
> * See if the config requires any additional preparation, e.g.
> * to adjust global state with pipes off. We need to do this
> @@ -11042,19 +11059,22 @@ done:
> if (ret && crtc->enabled)
> crtc->mode = *saved_mode;
>
> -out:
> kfree(pipe_config);
> kfree(saved_mode);
> return ret;
> }
>
> -static int intel_set_mode(struct drm_crtc *crtc,
> - struct drm_display_mode *mode,
> - int x, int y, struct drm_framebuffer *fb)
> +static int intel_set_mode_pipes(struct drm_crtc *crtc,
> + struct drm_display_mode *mode,
> + int x, int y, struct drm_framebuffer *fb,
> + unsigned modeset_pipes,
> + unsigned prepare_pipes,
> + unsigned disable_pipes)
> {
> int ret;
>
> - ret = __intel_set_mode(crtc, mode, x, y, fb);
> + ret = __intel_set_mode(crtc, mode, x, y, fb, modeset_pipes,
> + prepare_pipes, disable_pipes);
>
> if (ret == 0)
> intel_modeset_check_state(crtc->dev);
> @@ -11062,6 +11082,23 @@ static int intel_set_mode(struct drm_crtc *crtc,
> return ret;
> }
>
> +static int intel_set_mode(struct drm_crtc *crtc,
> + struct drm_display_mode *mode,
> + int x, int y, struct drm_framebuffer *fb)
> +{
> + unsigned modeset_pipes, prepare_pipes, disable_pipes;
> + int ret;
> +
> + ret = intel_modeset_compute_config(crtc, mode, fb,
> + &modeset_pipes, &prepare_pipes,
> + &disable_pipes);
> + if (ret)
> + return ret;
> +
> + return intel_set_mode_pipes(crtc, mode, x, y, fb, modeset_pipes,
> + prepare_pipes, disable_pipes);
> +}
> +
> void intel_crtc_restore_mode(struct drm_crtc *crtc)
> {
> intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, crtc->primary->fb);
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 1/6] drm/i915: factor out compute_config from __intel_set_mode
2014-10-29 14:30 ` [PATCH 1/6] drm/i915: factor out compute_config from __intel_set_mode Ander Conselvan de Oliveira
@ 2014-10-30 18:38 ` Jesse Barnes
2014-11-03 11:39 ` Daniel Vetter
0 siblings, 1 reply; 14+ messages in thread
From: Jesse Barnes @ 2014-10-30 18:38 UTC (permalink / raw)
To: Ander Conselvan de Oliveira; +Cc: intel-gfx
On Wed, 29 Oct 2014 16:30:43 +0200
Ander Conselvan de Oliveira <conselvan2@gmail.com> wrote:
> On 10/23/2014 09:50 PM, Jesse Barnes wrote:
> > This allows us to calculate the full pipe config before we do any
> > mode setting work.
> >
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 93
> > +++++++++++++++++++++++++----------- 1 file changed, 65
> > insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c index 6e5bc3c..d5f95e4 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -10910,45 +10910,62 @@ static void update_scanline_offset(struct
> > intel_crtc *crtc) crtc->scanline_offset = 1;
> > }
> >
> > +static int intel_modeset_compute_config(struct drm_crtc *crtc,
>
> s/drm_crtc/intel_crtc/
Since 2/3 of the functions this one calls take a drm_crtc I figured I'd
leave it for a big cleanup patch (maybe cocci could do it).
> > + /* Hack: Because we don't (yet) support global modeset on
> > multiple
> > + * crtcs, we don't keep track of the new mode for more
> > than one crtc.
> > + * Hence simply check whether any bit is set in
> > modeset_pipes in all the
> > + * pieces of code that are not yet converted to deal with
> > mutliple crtcs
> > + * changing their mode at the same time. */
>
> This comment seems out of place here since it refers to checking for
> set bits in modeset_pipes. I guess the important part of it is to
> explain the fact that only one new pipe config is obtained here, so
> maybe it can reworded to make that clearer.
>
> And there's another "if (modeset_pipes)" left in __intel_set_mode()
> to which this comment applies.
I tried to clear these up a bit, hopefully I didn't make things worse.
>
> > + pipe_config = intel_modeset_pipe_config(crtc, fb, mode);
> > + if (IS_ERR(pipe_config)) {
> > + ret = PTR_ERR(pipe_config);
> > + pipe_config = NULL;
> > +
> > + goto out;
>
> Nothing will use pipe_config after this point, so there's no need to
> set it to NULL.
Yep, fixed.
>
> > + }
> > + intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
> > + "[modeset]");
> > + to_intel_crtc(crtc)->new_config = pipe_config;
> > +
> > +out:
> > + return ret;
>
> There's nothing being done here, so we could avoid this goto dance
> and just use returns instead.
Yeah I just tend to use the "return only in one place" style, but I
have no strong preference if someone wants to change it.
>
> Anyway, mostly nit picks, so either way,
>
> Reviewed-by: Ander Conselvan de Oliveira
> <ander.conselvan.de.oliveira@intel.com>
Thanks, I'll post v2 shortly.
Jesse
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 1/6] drm/i915: factor out compute_config from __intel_set_mode
2014-10-30 18:38 ` Jesse Barnes
@ 2014-11-03 11:39 ` Daniel Vetter
0 siblings, 0 replies; 14+ messages in thread
From: Daniel Vetter @ 2014-11-03 11:39 UTC (permalink / raw)
To: Jesse Barnes; +Cc: intel-gfx
On Thu, Oct 30, 2014 at 11:38:20AM -0700, Jesse Barnes wrote:
> On Wed, 29 Oct 2014 16:30:43 +0200
> Ander Conselvan de Oliveira <conselvan2@gmail.com> wrote:
> > On 10/23/2014 09:50 PM, Jesse Barnes wrote:
> > > +static int intel_modeset_compute_config(struct drm_crtc *crtc,
> >
> > s/drm_crtc/intel_crtc/
>
> Since 2/3 of the functions this one calls take a drm_crtc I figured I'd
> leave it for a big cleanup patch (maybe cocci could do it).
Imo if it's driver-internal just do it, otherwise there will always be
hold-outs. And the &crtc->base doesn't really look all that ugly imo.
-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
^ permalink raw reply [flat|nested] 14+ messages in thread