[Intel-gfx] [i-g-t PATCH] README: add libkmod-dev and libprocps3-dev to dependencies
The list is perpetually out of date, but giving an idea of what the dependencies are is helpful. Signed-off-by: Jani Nikula --- README | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README b/README index d302af3f15dd..78a0a244d0ba 100644 --- a/README +++ b/README @@ -136,7 +136,9 @@ everything (package names may vary): gtk-doc-tools libcairo2-dev libdrm-dev + libkmod-dev libpciaccess-dev + libprocps3-dev libunwind-dev python-docutils x11proto-dri2-dev -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v4] lib/debugfs: Support new generic ABI for CRC capture
On Mon, 05 Dec 2016, Tomeu Vizoso wrote:
> The kernel has now a new debugfs ABI that can also allow capturing frame
> CRCs for drivers other than i915.
>
> Add alternative codepaths so the new ABI is used if the kernel is recent
> enough, and fall back to the legacy ABI if not.
>
> Signed-off-by: Tomeu Vizoso
...
> static int read_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out)
> {
> ssize_t bytes_read;
> - char buf[pipe_crc->buffer_len];
> + char buf[MAX_LINE_LEN + 1];
> + size_t read_len;
> +
> + if (pipe_crc->is_legacy)
> + read_len = LEGACY_LINE_LEN;
> + else
> + read_len = MAX_LINE_LEN;
>
> igt_set_timeout(5, "CRC reading");
> - bytes_read = read(pipe_crc->crc_fd, &buf, pipe_crc->line_len);
> + bytes_read = read(pipe_crc->crc_fd, &buf, read_len);
> igt_reset_timeout();
>
> if (bytes_read < 0 && errno == EAGAIN) {
> igt_assert(pipe_crc->flags & O_NONBLOCK);
> bytes_read = 0;
> - } else {
> - igt_assert_eq(bytes_read, pipe_crc->line_len);
> }
Leaving out the else branch leads to
igt_debugfs.c: In function 'read_crc':
igt_debugfs.c:604:5: error: array subscript is below array bounds
[-Werror=array-bounds]
buf[bytes_read] = '\0';
^
as bytes_read may end up being < 0.
BR,
Jani.
> buf[bytes_read] = '\0';
>
> - if (bytes_read && !pipe_crc_init_from_string(out, buf))
> + if (bytes_read && !pipe_crc_init_from_string(pipe_crc, out, buf))
> return -EINVAL;
>
> return bytes_read;
> @@ -530,15 +610,18 @@ void igt_pipe_crc_start(igt_pipe_crc_t *pipe_crc)
>
> igt_assert(igt_pipe_crc_do_start(pipe_crc));
>
> - /*
> - * For some no yet identified reason, the first CRC is bonkers. So
> - * let's just wait for the next vblank and read out the buggy result.
> - *
> - * On CHV sometimes the second CRC is bonkers as well, so don't trust
> - * that one either.
> - */
> - read_one_crc(pipe_crc, &crc);
> - read_one_crc(pipe_crc, &crc);
> + if (pipe_crc->is_legacy) {
> + /*
> + * For some no yet identified reason, the first CRC is
> + * bonkers. So let's just wait for the next vblank and read
> + * out the buggy result.
> + *
> + * On CHV sometimes the second CRC is bonkers as well, so
> + * don't trust that one either.
> + */
> + read_one_crc(pipe_crc, &crc);
> + read_one_crc(pipe_crc, &crc);
> + }
> }
>
> /**
> @@ -551,8 +634,15 @@ void igt_pipe_crc_stop(igt_pipe_crc_t *pipe_crc)
> {
> char buf[32];
>
> - sprintf(buf, "pipe %s none", kmstest_pipe_name(pipe_crc->pipe));
> - igt_assert_eq(write(pipe_crc->ctl_fd, buf, strlen(buf)), strlen(buf));
> + if (pipe_crc->is_legacy) {
> + sprintf(buf, "pipe %s none",
> + kmstest_pipe_name(pipe_crc->pipe));
> + igt_assert_eq(write(pipe_crc->ctl_fd, buf, strlen(buf)),
> + strlen(buf));
> + } else {
> + close(pipe_crc->crc_fd);
> + pipe_crc->crc_fd = -1;
> + }
> }
>
> /**
> diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
> index fb189322633f..4c6572cd244c 100644
> --- a/lib/igt_debugfs.h
> +++ b/lib/igt_debugfs.h
> @@ -62,6 +62,7 @@ bool igt_debugfs_search(const char *filename, const char
> *substring);
> */
> typedef struct _igt_pipe_crc igt_pipe_crc_t;
>
> +#define DRM_MAX_CRC_NR 10
> /**
> * igt_crc_t:
> * @frame: frame number of the capture CRC
> @@ -73,8 +74,9 @@ typedef struct _igt_pipe_crc igt_pipe_crc_t;
> */
> typedef struct {
> uint32_t frame;
> + bool has_valid_frame;
> int n_words;
> - uint32_t crc[5];
> + uint32_t crc[DRM_MAX_CRC_NR];
> } igt_crc_t;
>
> /**
> diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
> index 04d5a1345760..b106f9bd05ee 100644
> --- a/tests/kms_pipe_crc_basic.c
> +++ b/tests/kms_pipe_crc_basic.c
> @@ -49,6 +49,8 @@ static void test_bad_command(data_t *data, const char *cmd)
> size_t written;
>
> ctl = igt_debugfs_fopen("i915_display_crc_ctl", "r+");
> + igt_require(ctl);
> +
> written = fwrite(cmd, 1, strlen(cmd), ctl);
> fflush(ctl);
> igt_assert_eq(written, strlen(cmd));
> @@ -58,6 +60,30 @@ static void test_bad_command(data_t *data, const char *cmd)
> fclose(ctl);
> }
>
> +static void test_bad_source(data_t *data)
> +{
> + FILE *f;
> + size_t written;
> + const char *source = "foo";
> +
> + f = igt_debugfs_fopen("crtc-0/crc/control", "w");
> + if (!f) {
> + test_bad_command(data, "pipe A foo");
> + return;
> + }
> +
> + written = fwrite(source, 1, strlen(source), f);
> + fflush(f);
> + igt_assert_eq(written, strlen(source));
> + igt_assert(!ferror(f));
> + igt_assert(!errno);
> +
Re: [Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: Respect num_pipes when install or reset display IRQ
This patch shouldn't impact on platforms that have non-zero num_pipes. The warning happened to snb, which has non-zero num_pipes according to dmesg log "[drm:intel_modeset_init [i915]] 2 display pipes available" in https://intel-gfx-ci.01.org/CI/Patchwork_3383/fi-snb-2520m/dmesg-before.log. So the warning isn't a regression caused by this patch. According to the error message in dmesg log, it looks like it's the same issue reported by Bug 98228 - [BAT IVB] HDMI *ERROR* EDID checksum is invalid ( https://bugs.freedesktop.org/show_bug.cgi?id=98228) [ 413.875957] [drm:drm_edid_block_valid] *ERROR* EDID checksum is invalid, remainder is 63 Thanks, Elaine > -Original Message- > From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf > Of Patchwork > Sent: Friday, December 23, 2016 2:54 PM > To: Wang, Elaine > Cc: intel-gfx@lists.freedesktop.org > Subject: [Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: Respect num_pipes > when install or reset display IRQ > > == Series Details == > > Series: drm/i915: Respect num_pipes when install or reset display IRQ > URL : https://patchwork.freedesktop.org/series/17164/ > State : warning > > == Summary == > > Series 17164v1 drm/i915: Respect num_pipes when install or reset display > IRQ > https://patchwork.freedesktop.org/api/1.0/series/17164/revisions/1/mbox/ > > Test kms_flip: > Subgroup basic-plain-flip: > pass -> DMESG-WARN (fi-snb-2520m) > > fi-bdw-5557u total:246 pass:232 dwarn:0 dfail:0 fail:0 skip:14 > fi-bsw-n3050 total:246 pass:207 dwarn:0 dfail:0 fail:0 skip:39 > fi-bxt-j4205 total:246 pass:224 dwarn:0 dfail:0 fail:0 skip:22 > fi-bxt-t5700 total:82 pass:69 dwarn:0 dfail:0 fail:0 skip:12 > fi-byt-j1900 total:246 pass:219 dwarn:0 dfail:0 fail:0 skip:27 > fi-byt-n2820 total:246 pass:215 dwarn:0 dfail:0 fail:0 skip:31 > fi-hsw-4770 total:246 pass:227 dwarn:0 dfail:0 fail:0 skip:19 > fi-hsw-4770r total:246 pass:227 dwarn:0 dfail:0 fail:0 skip:19 > fi-ivb-3520m total:246 pass:225 dwarn:0 dfail:0 fail:0 skip:21 > fi-ivb-3770 total:246 pass:225 dwarn:0 dfail:0 fail:0 skip:21 > fi-kbl-7500u total:246 pass:225 dwarn:0 dfail:0 fail:0 skip:21 > fi-skl-6260u total:246 pass:233 dwarn:0 dfail:0 fail:0 skip:13 > fi-skl-6700hqtotal:246 pass:226 dwarn:0 dfail:0 fail:0 skip:20 > fi-skl-6700k total:246 pass:222 dwarn:3 dfail:0 fail:0 skip:21 > fi-skl-6770hqtotal:246 pass:233 dwarn:0 dfail:0 fail:0 skip:13 > fi-snb-2520m total:246 pass:214 dwarn:1 dfail:0 fail:0 skip:31 > fi-snb-2600 total:246 pass:214 dwarn:0 dfail:0 fail:0 skip:32 > > 7165fdc7f0b0b536557fcd0a222d083a901be57c drm-tip: 2016y-12m-22d-19h- > 42m-25s UTC integration manifest > 2728153 drm/i915: Respect num_pipes when install or reset display IRQ > > == Logs == > > For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3383/ > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Check num_pipes before initializing or calling display hooks
From: Elaine Wang
when num_pipes is zero, it indicates display doesn't exist, so there
is no need to initialize display hooks. And to avoid calling these
uninitialized display hooks, respect num_pipes at the beginning of
intel_modeset_init_hw.
intel_init_pm() calls FBC init function and then initializes
water mark hooks. Both aren't needed when display doesn't exist. Add
checking num_pipes at the beginning of intel_init_pm().
v2: Move one check from caller to callee for consistency.
v3: Addressed Jani's Review comments(minimize the amount of the checks)
- Assign nop_init_clock_gating() to the clock gating hook when
num_pipes is zero.
Cc: Chris Wilson
Cc: Joonas Lahtinen
Cc: Jani Nikula
Signed-off-by: Elaine Wang
---
drivers/gpu/drm/i915/intel_display.c | 6 ++
drivers/gpu/drm/i915/intel_pm.c | 8 +++-
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index d8effd4..eb180df 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -16161,6 +16161,9 @@ static void intel_atomic_state_free(struct
drm_atomic_state *state)
*/
void intel_init_display_hooks(struct drm_i915_private *dev_priv)
{
+ if (INTEL_INFO(dev_priv)->num_pipes == 0)
+ return;
+
if (INTEL_INFO(dev_priv)->gen >= 9) {
dev_priv->display.get_pipe_config = haswell_get_pipe_config;
dev_priv->display.get_initial_plane_config =
@@ -16543,6 +16546,9 @@ void intel_modeset_init_hw(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
+ if (INTEL_INFO(dev_priv)->num_pipes == 0)
+ return;
+
intel_update_cdclk(dev_priv);
dev_priv->atomic_cdclk_freq = dev_priv->cdclk_freq;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 338b542..bf63ba5 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7642,7 +7642,10 @@ static void nop_init_clock_gating(struct
drm_i915_private *dev_priv)
*/
void intel_init_clock_gating_hooks(struct drm_i915_private *dev_priv)
{
- if (IS_SKYLAKE(dev_priv))
+
+ if (INTEL_INFO(dev_priv)->num_pipes == 0)
+ dev_priv->display.init_clock_gating = nop_init_clock_gating;
+ else if (IS_SKYLAKE(dev_priv))
dev_priv->display.init_clock_gating = skylake_init_clock_gating;
else if (IS_KABYLAKE(dev_priv))
dev_priv->display.init_clock_gating =
kabylake_init_clock_gating;
@@ -7683,6 +7686,9 @@ void intel_init_clock_gating_hooks(struct
drm_i915_private *dev_priv)
/* Set up chip specific power management-related functions */
void intel_init_pm(struct drm_i915_private *dev_priv)
{
+ if (INTEL_INFO(dev_priv)->num_pipes == 0)
+ return;
+
intel_fbc_init(dev_priv);
/* For cxsr */
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 07/14] drm/i915: Start moving the cdclk stuff into a distinct state structure
On Thu, 2016-12-22 at 16:33 +0200, Ville Syrjälä wrote:
> On Thu, Dec 22, 2016 at 04:14:40PM +0200, Ander Conselvan De Oliveira wrote:
> >
> > On Mon, 2016-12-19 at 19:28 +0200, ville.syrj...@linux.intel.com wrote:
> > >
> > > From: Ville Syrjälä
> > >
> > > Introduce intel_cdclk state which for now will track the cdclk
> > > frequency, the vco frequency and the reference frequency (not sure we
> > > want the last one, but I put it there anyway). We'll also make the
> > > .get_cdclk() function fill out this state structure rather than
> > > just returning the current cdclk frequency.
> > >
> > > One immediate benefit is that calling .get_cdclk() will no longer
> > > clobber state stored under dev_priv unless ex[plicitly told to do
> > Typo: ex[plicity
> >
> > >
> > > so. Previously it clobbered the vco and reference clocks stored
> > > there on some platforms.
> > >
> > > We'll expand the use of this structure to actually precomputing the
> > > state and whatnot later.
> > >
> > > v2: Constify intel_cdclk_state_compare()
> > >
> > > Signed-off-by: Ville Syrjälä
> > A couple more comments beloew, but either way,
> >
> > Reviewed-by: Ander Conselvan de Oliveira
> >
> > >
> > > ---
> > > drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
> > > drivers/gpu/drm/i915/i915_drv.h | 14 +-
> > > drivers/gpu/drm/i915/intel_audio.c | 2 +-
> > > drivers/gpu/drm/i915/intel_cdclk.c | 359
> > > ++--
> > > drivers/gpu/drm/i915/intel_display.c| 18 +-
> > > drivers/gpu/drm/i915/intel_dp.c | 2 +-
> > > drivers/gpu/drm/i915/intel_drv.h| 3 +
> > > drivers/gpu/drm/i915/intel_fbc.c| 2 +-
> > > drivers/gpu/drm/i915/intel_panel.c | 4 +-
> > > drivers/gpu/drm/i915/intel_runtime_pm.c | 5 +-
> > > 10 files changed, 240 insertions(+), 171 deletions(-)
> > >
[...]
@@ -629,12 +668,13 @@ static int skl_calc_cdclk(int max_pixclk, int vco)
> > > }
> > >
> > > -static void skl_dpll0_update(struct drm_i915_private *dev_priv)
> > > +static void skl_dpll0_update(struct drm_i915_private *dev_priv,
> > > + struct intel_cdclk_state *cdclk_state)
> > > {
> > > u32 val;
> > >
> > > - dev_priv->cdclk_pll.ref = 24000;
> > > - dev_priv->cdclk_pll.vco = 0;
> > > + cdclk_state->ref = 24000;
> > > + cdclk_state->vco = 0;
> > >
> > > val = I915_READ(LCPLL1_CTL);
> > > if ((val & LCPLL_PLL_ENABLE) == 0)
> > > @@ -656,11 +696,11 @@ static void skl_dpll0_update(struct
> > > drm_i915_private *dev_priv)
> > > case DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1350, SKL_DPLL0):
> > > case DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1620, SKL_DPLL0):
> > > case DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_2700, SKL_DPLL0):
> > > - dev_priv->cdclk_pll.vco = 810;
> > > + cdclk_state->vco = 810;
> > > break;
> > > case DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1080, SKL_DPLL0):
> > > case DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_2160, SKL_DPLL0):
> > > - dev_priv->cdclk_pll.vco = 864;
> > > + cdclk_state->vco = 864;
> > > break;
> > > default:
> > > MISSING_CASE(val & DPLL_CTRL1_LINK_RATE_MASK(SKL_DPLL0));
> > This function is a bit funny now, since it sets up the cdclk state based on
> > current pll0 programming, instead of looking at an atomic state.
> We should probably rename it to somehting more descriptive. skl_get_dpll0()
> maybe. Or maybe skl_dpll0_get_config to match the .get_config()
> nomenclature used for eg. encoders.
Hmm, I think the problem is that this have a mix of get_hw_state() and
get_config(). We actually have skl_ddi_dpll0_get_hw_state(), but it wouldn't be
trivial to make that code reusable for the cdclk stuff.
Maybe it should be two separate functions, one that gets the hardware state and
another for choosing the vco. Then
vco = skl_cdclk_vco(skl_dpll0_get_hw_state());
directly in skl_get_cdclk() ? I'm not really sure, maybe just leave the way it
is for now.
> Though if we go there then
> .get_cdclk() might need to be renamed to .cdclk_get_config or something.
.cdclk_get_hw_state() ?
Ander
>
> >
> > But so far it
> > is only used to read the current cdclk, so there's no harm.
> >
> > >
> > > @@ -668,46 +708,57 @@ static void skl_dpll0_update(struct
> > > drm_i915_private *dev_priv)
> > > }
> > > }
> > >
> > > -static int skl_get_cdclk(struct drm_i915_private *dev_priv)
> > > +static void skl_get_cdclk(struct drm_i915_private *dev_priv,
> > > + struct intel_cdclk_state *cdclk_state)
> > > {
> > > u32 cdctl;
> > >
> > > - skl_dpll0_update(dev_priv);
> > > + skl_dpll0_update(dev_priv, cdclk_state);
> > > +
> > > + cdclk_state->cdclk = cdclk_state->ref;
> > >
> > > - if (dev_priv->cdclk_pll.vco == 0)
> > > - return dev_priv->cdclk_pll.ref;
> > > + if (cdclk_state->vco == 0)
> > > + return;
> > >
> > > cdctl = I915_READ(CDCLK_CTL);
> > >
> > > -
Re: [Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: Respect num_pipes when install or reset display IRQ
HI, > -Original Message- > From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf > Of Wang, Elaine > Sent: Friday, December 23, 2016 11:04 AM > To: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: Respect num_pipes > when install or reset display IRQ > > This patch shouldn't impact on platforms that have non-zero num_pipes. The > warning happened to snb, which has non-zero num_pipes according to > dmesg log "[drm:intel_modeset_init [i915]] 2 display pipes available" in > https://intel-gfx-ci.01.org/CI/Patchwork_3383/fi-snb-2520m/dmesg- > before.log. So the warning isn't a regression caused by this patch. > > According to the error message in dmesg log, it looks like it's the same issue > reported by Bug 98228 - [BAT IVB] HDMI *ERROR* EDID checksum is invalid ( > https://bugs.freedesktop.org/show_bug.cgi?id=98228) Almost as we have own for SNB too: https://bugs.freedesktop.org/show_bug.cgi?id=98625 But yes, is known issue. > > [ 413.875957] [drm:drm_edid_block_valid] *ERROR* EDID checksum is > invalid, remainder is 63 > > Thanks, > Elaine > > -Original Message- > > From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On > > Behalf Of Patchwork > > Sent: Friday, December 23, 2016 2:54 PM > > To: Wang, Elaine > > Cc: intel-gfx@lists.freedesktop.org > > Subject: [Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: Respect > > num_pipes when install or reset display IRQ > > > > == Series Details == > > > > Series: drm/i915: Respect num_pipes when install or reset display IRQ > > URL : https://patchwork.freedesktop.org/series/17164/ > > State : warning > > > > == Summary == > > > > Series 17164v1 drm/i915: Respect num_pipes when install or reset > > display IRQ > > https://patchwork.freedesktop.org/api/1.0/series/17164/revisions/1/mbo > > x/ > > > > Test kms_flip: > > Subgroup basic-plain-flip: > > pass -> DMESG-WARN (fi-snb-2520m) > > > > fi-bdw-5557u total:246 pass:232 dwarn:0 dfail:0 fail:0 skip:14 > > fi-bsw-n3050 total:246 pass:207 dwarn:0 dfail:0 fail:0 skip:39 > > fi-bxt-j4205 total:246 pass:224 dwarn:0 dfail:0 fail:0 skip:22 > > fi-bxt-t5700 total:82 pass:69 dwarn:0 dfail:0 fail:0 skip:12 > > fi-byt-j1900 total:246 pass:219 dwarn:0 dfail:0 fail:0 skip:27 > > fi-byt-n2820 total:246 pass:215 dwarn:0 dfail:0 fail:0 skip:31 > > fi-hsw-4770 total:246 pass:227 dwarn:0 dfail:0 fail:0 skip:19 > > fi-hsw-4770r total:246 pass:227 dwarn:0 dfail:0 fail:0 skip:19 > > fi-ivb-3520m total:246 pass:225 dwarn:0 dfail:0 fail:0 skip:21 > > fi-ivb-3770 total:246 pass:225 dwarn:0 dfail:0 fail:0 skip:21 > > fi-kbl-7500u total:246 pass:225 dwarn:0 dfail:0 fail:0 skip:21 > > fi-skl-6260u total:246 pass:233 dwarn:0 dfail:0 fail:0 skip:13 > > fi-skl-6700hqtotal:246 pass:226 dwarn:0 dfail:0 fail:0 skip:20 > > fi-skl-6700k total:246 pass:222 dwarn:3 dfail:0 fail:0 skip:21 > > fi-skl-6770hqtotal:246 pass:233 dwarn:0 dfail:0 fail:0 skip:13 > > fi-snb-2520m total:246 pass:214 dwarn:1 dfail:0 fail:0 skip:31 > > fi-snb-2600 total:246 pass:214 dwarn:0 dfail:0 fail:0 skip:32 > > > > 7165fdc7f0b0b536557fcd0a222d083a901be57c drm-tip: 2016y-12m-22d-19h- > > 42m-25s UTC integration manifest > > 2728153 drm/i915: Respect num_pipes when install or reset display IRQ > > > > == Logs == > > > > For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3383/ Jani Saarinen Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Check num_pipes before initializing or calling display hooks
== Series Details == Series: drm/i915: Check num_pipes before initializing or calling display hooks URL : https://patchwork.freedesktop.org/series/17173/ State : success == Summary == Series 17173v1 drm/i915: Check num_pipes before initializing or calling display hooks https://patchwork.freedesktop.org/api/1.0/series/17173/revisions/1/mbox/ fi-bdw-5557u total:246 pass:232 dwarn:0 dfail:0 fail:0 skip:14 fi-bsw-n3050 total:246 pass:207 dwarn:0 dfail:0 fail:0 skip:39 fi-bxt-j4205 total:246 pass:224 dwarn:0 dfail:0 fail:0 skip:22 fi-bxt-t5700 total:82 pass:69 dwarn:0 dfail:0 fail:0 skip:12 fi-byt-j1900 total:246 pass:219 dwarn:0 dfail:0 fail:0 skip:27 fi-byt-n2820 total:246 pass:215 dwarn:0 dfail:0 fail:0 skip:31 fi-hsw-4770 total:246 pass:227 dwarn:0 dfail:0 fail:0 skip:19 fi-hsw-4770r total:246 pass:227 dwarn:0 dfail:0 fail:0 skip:19 fi-ivb-3520m total:246 pass:225 dwarn:0 dfail:0 fail:0 skip:21 fi-ivb-3770 total:246 pass:225 dwarn:0 dfail:0 fail:0 skip:21 fi-kbl-7500u total:246 pass:225 dwarn:0 dfail:0 fail:0 skip:21 fi-skl-6260u total:246 pass:233 dwarn:0 dfail:0 fail:0 skip:13 fi-skl-6700hqtotal:246 pass:226 dwarn:0 dfail:0 fail:0 skip:20 fi-skl-6700k total:246 pass:222 dwarn:3 dfail:0 fail:0 skip:21 fi-skl-6770hqtotal:246 pass:233 dwarn:0 dfail:0 fail:0 skip:13 fi-snb-2520m total:246 pass:215 dwarn:0 dfail:0 fail:0 skip:31 fi-snb-2600 total:246 pass:214 dwarn:0 dfail:0 fail:0 skip:32 7165fdc7f0b0b536557fcd0a222d083a901be57c drm-tip: 2016y-12m-22d-19h-42m-25s UTC integration manifest b906d67 drm/i915: Check num_pipes before initializing or calling display hooks == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3384/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 08/14] drm/i915: Track full cdclk state for the logical and actual cdclk frequencies
On Mon, 2016-12-19 at 19:28 +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> The current dev_cdclk vs. cdclk vs. atomic_cdclk_freq is quite a mess.
> So here I'm introducing the "actual" and "logical" naming for our
> cdclk state. "actual" is what we'll bash into the hardware and "logical"
> is what everyone should use for state computaion/checking and whatnot.
> We'll track both using the intel_cdclk_state as both will need other
> differing parameters than just the actual cdclk frequency.
>
> While doing that we can at the same time unify the appearance of the
> .modeset_calc_cdclk() implementations a little bit.
>
> v2: Commit dev_priv->cdclk.actual since that already has the
> new state by the time .modeset_commit_cdclk() is called.
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/i915_drv.h | 13 ++--
> drivers/gpu/drm/i915/intel_cdclk.c | 123
> ++-
> drivers/gpu/drm/i915/intel_display.c | 39 ++-
> drivers/gpu/drm/i915/intel_dp.c | 2 +-
> drivers/gpu/drm/i915/intel_drv.h | 24 ---
> drivers/gpu/drm/i915/intel_pm.c | 4 +-
> 6 files changed, 121 insertions(+), 84 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a4f1231ff8ca..b5a8d0f4cfbd 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2237,18 +2237,19 @@ struct drm_i915_private {
> unsigned int skl_preferred_vco_freq;
> unsigned int max_cdclk_freq;
>
> - /*
> - * For reading holding any crtc lock is sufficient,
> - * for writing must hold all of them.
> - */
> - unsigned int atomic_cdclk_freq;
> -
> unsigned int max_dotclk_freq;
> unsigned int rawclk_freq;
> unsigned int hpll_freq;
> unsigned int czclk_freq;
>
> struct {
> + /*
> + * The current logical cdclk state.
> + * For reading holding any crtc lock is sufficient,
> + * for writing must hold all of them.
> + */
> + struct intel_cdclk_state logical;
> + struct intel_cdclk_state actual;
> struct intel_cdclk_state hw;
It would be great if all three fields were documented, including an example
configuration where the logical and actual fields differ and what which fields
hold, to make the distinction clear.
> } cdclk;
>
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c
> b/drivers/gpu/drm/i915/intel_cdclk.c
> index 13c3b473..08ae3b78b8ed 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -1368,12 +1368,26 @@ static int vlv_modeset_calc_cdclk(struct
> drm_atomic_state *state)
> int max_pixclk = intel_max_pixel_rate(state);
> struct intel_atomic_state *intel_state =
> to_intel_atomic_state(state);
> + int cdclk;
> +
> + cdclk = vlv_calc_cdclk(dev_priv, max_pixclk);
> +
> + if (cdclk > dev_priv->max_cdclk_freq) {
> + DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
> + cdclk, dev_priv->max_cdclk_freq);
> + return -EINVAL;
> + }
>
> - intel_state->cdclk = intel_state->dev_cdclk =
> - vlv_calc_cdclk(dev_priv, max_pixclk);
> + intel_state->cdclk.logical.cdclk = cdclk;
>
> - if (!intel_state->active_crtcs)
> - intel_state->dev_cdclk = vlv_calc_cdclk(dev_priv, 0);
> + if (!intel_state->active_crtcs) {
> + cdclk = vlv_calc_cdclk(dev_priv, 0);
> +
> + intel_state->cdclk.actual.cdclk = cdclk;
> + } else {
> + intel_state->cdclk.actual =
> + intel_state->cdclk.logical;
> + }
>
> return 0;
> }
> @@ -1382,9 +1396,7 @@ static void vlv_modeset_commit_cdclk(struct
> drm_atomic_state *old_state)
> {
> struct drm_device *dev = old_state->dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> - struct intel_atomic_state *old_intel_state =
> - to_intel_atomic_state(old_state);
> - unsigned int req_cdclk = old_intel_state->dev_cdclk;
> + unsigned int req_cdclk = dev_priv->cdclk.actual.cdclk;
>
> /*
> * FIXME: We can end up here with all power domains off, yet
> @@ -1426,9 +1438,16 @@ static int bdw_modeset_calc_cdclk(struct
> drm_atomic_state *state)
> return -EINVAL;
> }
>
> - intel_state->cdclk = intel_state->dev_cdclk = cdclk;
> - if (!intel_state->active_crtcs)
> - intel_state->dev_cdclk = bdw_calc_cdclk(0);
> + intel_state->cdclk.logical.cdclk = cdclk;
> +
> + if (!intel_state->active_crtcs) {
> + cdclk = bdw_calc_cdclk(0);
> +
> + intel_state->cdclk.actual.cdclk = cdclk;
> + } else {
> + intel_state->cdclk.actual =
> + intel_state->cdclk.logical;
> + }
>
> return 0;
> }
Re: [Intel-gfx] [PATCH v2 09/14] drm/i915: Pass dev_priv to remainder of the cdclk functions
On Mon, 2016-12-19 at 19:28 +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Clean up the dev vs. dev_priv straggles that are making things
> look inconsistentt.
>
> v2: Deal with churn
>
> Signed-off-by: Ville Syrjälä
Reviewed-by: Ander Conselvan de Oliveira
> ---
> drivers/gpu/drm/i915/intel_cdclk.c | 25 ++---
> 1 file changed, 10 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c
> b/drivers/gpu/drm/i915/intel_cdclk.c
> index 08ae3b78b8ed..2e998fa087a9 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -433,9 +433,8 @@ static void vlv_program_pfi_credits(struct
> drm_i915_private *dev_priv)
> WARN_ON(I915_READ(GCI_CONTROL) & PFI_CREDIT_RESEND);
> }
>
> -static void vlv_set_cdclk(struct drm_device *dev, int cdclk)
> +static void vlv_set_cdclk(struct drm_i915_private *dev_priv, int cdclk)
> {
> - struct drm_i915_private *dev_priv = to_i915(dev);
> u32 val, cmd;
>
> if (cdclk >= 32) /* jump to highest voltage for 400MHz too */
> @@ -496,9 +495,8 @@ static void vlv_set_cdclk(struct drm_device *dev, int
> cdclk)
> intel_update_cdclk(dev_priv);
> }
>
> -static void chv_set_cdclk(struct drm_device *dev, int cdclk)
> +static void chv_set_cdclk(struct drm_i915_private *dev_priv, int cdclk)
> {
> - struct drm_i915_private *dev_priv = to_i915(dev);
> u32 val, cmd;
>
> switch (cdclk) {
> @@ -566,9 +564,8 @@ static void bdw_get_cdclk(struct drm_i915_private
> *dev_priv,
> cdclk_state->cdclk = 675000;
> }
>
> -static void bdw_set_cdclk(struct drm_device *dev, int cdclk)
> +static void bdw_set_cdclk(struct drm_i915_private *dev_priv, int cdclk)
> {
> - struct drm_i915_private *dev_priv = to_i915(dev);
> uint32_t val, data;
> int ret;
>
> @@ -1363,8 +1360,7 @@ static int intel_max_pixel_rate(struct drm_atomic_state
> *state)
>
> static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state)
> {
> - struct drm_device *dev = state->dev;
> - struct drm_i915_private *dev_priv = to_i915(dev);
> + struct drm_i915_private *dev_priv = to_i915(state->dev);
> int max_pixclk = intel_max_pixel_rate(state);
> struct intel_atomic_state *intel_state =
> to_intel_atomic_state(state);
> @@ -1394,8 +1390,7 @@ static int vlv_modeset_calc_cdclk(struct
> drm_atomic_state *state)
>
> static void vlv_modeset_commit_cdclk(struct drm_atomic_state *old_state)
> {
> - struct drm_device *dev = old_state->dev;
> - struct drm_i915_private *dev_priv = to_i915(dev);
> + struct drm_i915_private *dev_priv = to_i915(old_state->dev);
> unsigned int req_cdclk = dev_priv->cdclk.actual.cdclk;
>
> /*
> @@ -1410,9 +1405,9 @@ static void vlv_modeset_commit_cdclk(struct
> drm_atomic_state *old_state)
> intel_display_power_get(dev_priv, POWER_DOMAIN_PIPE_A);
>
> if (IS_CHERRYVIEW(dev_priv))
> - chv_set_cdclk(dev, req_cdclk);
> + chv_set_cdclk(dev_priv, req_cdclk);
> else
> - vlv_set_cdclk(dev, req_cdclk);
> + vlv_set_cdclk(dev_priv, req_cdclk);
>
> vlv_program_pfi_credits(dev_priv);
>
> @@ -1454,10 +1449,10 @@ static int bdw_modeset_calc_cdclk(struct
> drm_atomic_state *state)
>
> static void bdw_modeset_commit_cdclk(struct drm_atomic_state *old_state)
> {
> - struct drm_device *dev = old_state->dev;
> - unsigned int req_cdclk = to_i915(dev)->cdclk.actual.cdclk;
> + struct drm_i915_private *dev_priv = to_i915(old_state->dev);
> + unsigned int req_cdclk = dev_priv->cdclk.actual.cdclk;
>
> - bdw_set_cdclk(dev, req_cdclk);
> + bdw_set_cdclk(dev_priv, req_cdclk);
> }
>
> static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: rewrite FBC's atomic CRTC-choosing code
Ville pointed out that intel_fbc_choose_crtc() is iterating over all
planes instead of just the primary planes. There are no real
consequences of this problem for HSW+, and for the other platforms it
just means that in some obscure multi-screen cases we'll keep FBC
disabled when we could have enabled it. Still, iterating over all
planes doesn't seem to be the best thing to do.
My initial idea was to just add a check for plane->type and be done,
but then I realized that in commits not involving the primary plane we
would reset crtc_state->enable_fbc back to false even when FBC is
enabled. That also wouldn't result in a bug due to the way the
enable_fbc variable is checked, but, still, our code can be better
than this.
So I went for the solution that involves tracking enable_fbc in the
primary plane state instead of the CRTC state. This way, if a commit
doesn't involve the primary plane for the CRTC we won't be resetting
enable_fbc back to false, so the variable will always reflect the
reality. And this change also makes more sense since FBC is actually
tied to the single plane and not the full pipe. As a bonus, we only
iterate over the CRTCs instead of iterating over all planes.
v2:
But Ville pointed that this only works properly if we have a single
commit with multiple CRTCs. If we have multiple parallel commits, each
with its own CRTC, we'll end with enable_fbc set to true in more than
one plane.
So the solution was to rename enable_fbc to can_enable_fbc. If we just
did the rename as the patch was, we'd end up with a single plane with
can_enable_fbc on commits involving multiple CRTCs: we'd choose the
best one, but we'd still end up with a variable that doesn't 100%
reflect reality.
So in the end I adopted Ville's suggestion of the fbc_score variable.
And then, instead of checking the score at intel_fbc_choose_crtc()
it should be possible to check for the score at intel_fbc_enable().
This allows us to simplify intel_fbc_choose_crtc() to the point where
it only needs to look at the given plane in order to assign its score
instead of looking at every primary plane on the same commit.
We still only set scores of 0 and 1 and we don't really do the
score-checking loop.
v3: Rebase.
Cc: Ville Syrjälä
Reported-by: Ville Syrjälä
Signed-off-by: Paulo Zanoni
---
drivers/gpu/drm/i915/intel_display.c | 3 +-
drivers/gpu/drm/i915/intel_drv.h | 8 ++--
drivers/gpu/drm/i915/intel_fbc.c | 85
3 files changed, 35 insertions(+), 61 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index d8effd4..92527d6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14191,7 +14191,6 @@ static int intel_atomic_check(struct drm_device *dev,
if (ret)
return ret;
- intel_fbc_choose_crtc(dev_priv, state);
return calc_watermark_data(state);
}
@@ -14966,6 +14965,8 @@ intel_check_primary_plane(struct drm_plane *plane,
return ret;
}
+ intel_fbc_check_plane(state);
+
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 025e4c8..7430082 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -406,6 +406,9 @@ struct intel_plane_state {
*/
int scaler_id;
+ /* 0: not suitable for FBC, 1+: suitable for FBC, more is better. */
+ unsigned int fbc_score;
+
struct drm_intel_sprite_colorkey ckey;
};
@@ -652,8 +655,6 @@ struct intel_crtc_state {
bool ips_enabled;
- bool enable_fbc;
-
bool double_wide;
int pbn;
@@ -1535,8 +1536,7 @@ static inline void intel_fbdev_restore_mode(struct
drm_device *dev)
#endif
/* intel_fbc.c */
-void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv,
- struct drm_atomic_state *state);
+void intel_fbc_check_plane(struct intel_plane_state *plane_state);
bool intel_fbc_is_active(struct drm_i915_private *dev_priv);
void intel_fbc_pre_update(struct intel_crtc *crtc,
struct intel_crtc_state *crtc_state,
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 26a81a9..ceda6f4 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -1040,68 +1040,32 @@ void intel_fbc_flush(struct drm_i915_private *dev_priv,
}
/**
- * intel_fbc_choose_crtc - select a CRTC to enable FBC on
- * @dev_priv: i915 device instance
- * @state: the atomic state structure
+ * intel_fbc_check_plane - check plane for FBC suitability
+ * @plane_state: the plane state
*
- * This function looks at the proposed state for CRTCs and planes, then chooses
- * which pipe is going to have FBC by setting intel_crtc_state->enable_fbc to
- * true.
+ * This function should set fbc_score based on how suitable the plane is for
+ * FBC. For now the o
[Intel-gfx] [PATCH 1/2] drm/i915: enable FBC on gen9+ too
Gen9+ platforms have been seeing a lot of screen flickerings and
underruns, so I never felt comfortable in enabling FBC on these
platforms since I didn't want to throw yet another feature on top of
the already complex problem. We now have code that automatically
disables FBC if we ever get an underrun, and the screen flickerings
seem to be mostly gone, so it may be a good time to try to finally
enable FBC by default on the newer platforms.
Besides, BDW FBC has been working fine over the year, which gives me a
little more confidence now.
For a little more information, please refer to commit a98ee79317b4
("drm/i915/fbc: enable FBC by default on HSW and BDW").
v2: Enable not only on SKL, but for everything new (Daniel).
v3: Rebase after the intel_sanitize_fbc_option() change.
v4: New rebase after 8 months, drop expired R-B tags.
Signed-off-by: Paulo Zanoni
---
drivers/gpu/drm/i915/intel_fbc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Daniel gave me a R-B for this patch many months ago, but I think
we can consider it as expired now.
QA just went through a round of testing and confirmed 100% pass rate
for SKL/KBL/BXT (VIZ-7181).
Besides, we do have at least one user with FBC enabled on SKL and
reporting possible issues, as we saw on #98213, which is already
fixed.
The problem that I've been discussing with Chris and Daniel is not a
user visible bug since it just keeps FBC deactivated if the client
doesn't issue the dirtyfb calls, and from what I can see, current user
space is fine.
I have a pending patch for an improvement on the CRTC-choosing code,
but that's not a bug fix and doesn't change how HSW+ behaves. It's
patch 2 of this series.
I think we can also consider reenabling FBC on HSW due to the
confirmation I got some time ago that the HSW regression is gone (both
bugs from c7f7e2feffb0 have confirmed that some patches that are
alredy merged fixed the problem without reverting HSW support). But
I won't propose this until we get to understand what's going on with
bug #99169.
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 9aec63b..26a81a9 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -1317,7 +1317,7 @@ static int intel_sanitize_fbc_option(struct
drm_i915_private *dev_priv)
if (!HAS_FBC(dev_priv))
return 0;
- if (IS_BROADWELL(dev_priv))
+ if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9)
return 1;
return 0;
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 07/14] drm/i915: Start moving the cdclk stuff into a distinct state structure
On Fri, Dec 23, 2016 at 11:09:22AM +0200, Ander Conselvan De Oliveira wrote:
> On Thu, 2016-12-22 at 16:33 +0200, Ville Syrjälä wrote:
> > On Thu, Dec 22, 2016 at 04:14:40PM +0200, Ander Conselvan De Oliveira wrote:
> > >
> > > On Mon, 2016-12-19 at 19:28 +0200, ville.syrj...@linux.intel.com wrote:
> > > >
> > > > From: Ville Syrjälä
> > > >
> > > > Introduce intel_cdclk state which for now will track the cdclk
> > > > frequency, the vco frequency and the reference frequency (not sure we
> > > > want the last one, but I put it there anyway). We'll also make the
> > > > .get_cdclk() function fill out this state structure rather than
> > > > just returning the current cdclk frequency.
> > > >
> > > > One immediate benefit is that calling .get_cdclk() will no longer
> > > > clobber state stored under dev_priv unless ex[plicitly told to do
> > > Typo: ex[plicity
> > >
> > > >
> > > > so. Previously it clobbered the vco and reference clocks stored
> > > > there on some platforms.
> > > >
> > > > We'll expand the use of this structure to actually precomputing the
> > > > state and whatnot later.
> > > >
> > > > v2: Constify intel_cdclk_state_compare()
> > > >
> > > > Signed-off-by: Ville Syrjälä
> > > A couple more comments beloew, but either way,
> > >
> > > Reviewed-by: Ander Conselvan de Oliveira
> > >
> > > >
> > > > ---
> > > > drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
> > > > drivers/gpu/drm/i915/i915_drv.h | 14 +-
> > > > drivers/gpu/drm/i915/intel_audio.c | 2 +-
> > > > drivers/gpu/drm/i915/intel_cdclk.c | 359
> > > > ++--
> > > > drivers/gpu/drm/i915/intel_display.c| 18 +-
> > > > drivers/gpu/drm/i915/intel_dp.c | 2 +-
> > > > drivers/gpu/drm/i915/intel_drv.h| 3 +
> > > > drivers/gpu/drm/i915/intel_fbc.c| 2 +-
> > > > drivers/gpu/drm/i915/intel_panel.c | 4 +-
> > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 5 +-
> > > > 10 files changed, 240 insertions(+), 171 deletions(-)
> > > >
>
> [...]
>
> @@ -629,12 +668,13 @@ static int skl_calc_cdclk(int max_pixclk, int vco)
> > > > }
> > > >
> > > > -static void skl_dpll0_update(struct drm_i915_private *dev_priv)
> > > > +static void skl_dpll0_update(struct drm_i915_private *dev_priv,
> > > > + struct intel_cdclk_state *cdclk_state)
> > > > {
> > > > u32 val;
> > > >
> > > > - dev_priv->cdclk_pll.ref = 24000;
> > > > - dev_priv->cdclk_pll.vco = 0;
> > > > + cdclk_state->ref = 24000;
> > > > + cdclk_state->vco = 0;
> > > >
> > > > val = I915_READ(LCPLL1_CTL);
> > > > if ((val & LCPLL_PLL_ENABLE) == 0)
> > > > @@ -656,11 +696,11 @@ static void skl_dpll0_update(struct
> > > > drm_i915_private *dev_priv)
> > > > case DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1350, SKL_DPLL0):
> > > > case DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1620, SKL_DPLL0):
> > > > case DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_2700, SKL_DPLL0):
> > > > - dev_priv->cdclk_pll.vco = 810;
> > > > + cdclk_state->vco = 810;
> > > > break;
> > > > case DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1080, SKL_DPLL0):
> > > > case DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_2160, SKL_DPLL0):
> > > > - dev_priv->cdclk_pll.vco = 864;
> > > > + cdclk_state->vco = 864;
> > > > break;
> > > > default:
> > > > MISSING_CASE(val &
> > > > DPLL_CTRL1_LINK_RATE_MASK(SKL_DPLL0));
> > > This function is a bit funny now, since it sets up the cdclk state based
> > > on
> > > current pll0 programming, instead of looking at an atomic state.
> > We should probably rename it to somehting more descriptive. skl_get_dpll0()
> > maybe. Or maybe skl_dpll0_get_config to match the .get_config()
> > nomenclature used for eg. encoders.
>
> Hmm, I think the problem is that this have a mix of get_hw_state() and
> get_config(). We actually have skl_ddi_dpll0_get_hw_state(), but it wouldn't
> be
> trivial to make that code reusable for the cdclk stuff.
>
> Maybe it should be two separate functions, one that gets the hardware state
> and
> another for choosing the vco.
This function only exists to read the hw state.
> Then
>
> vco = skl_cdclk_vco(skl_dpll0_get_hw_state());
>
> directly in skl_get_cdclk() ? I'm not really sure, maybe just leave the way it
> is for now.
>
> > Though if we go there then
> > .get_cdclk() might need to be renamed to .cdclk_get_config or something.
>
> .cdclk_get_hw_state() ?
.get_hw_state() is the thing which tells us if something is enabled,
.get_config() is the thing that actually reads out the full state. Or at
least that's how it used to be.
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listi
Re: [Intel-gfx] [PATCH 2/2] drm/i915: rewrite FBC's atomic CRTC-choosing code
On Fri, Dec 23, 2016 at 10:23:59AM -0200, Paulo Zanoni wrote:
> Ville pointed out that intel_fbc_choose_crtc() is iterating over all
> planes instead of just the primary planes. There are no real
> consequences of this problem for HSW+, and for the other platforms it
> just means that in some obscure multi-screen cases we'll keep FBC
> disabled when we could have enabled it. Still, iterating over all
> planes doesn't seem to be the best thing to do.
>
> My initial idea was to just add a check for plane->type and be done,
> but then I realized that in commits not involving the primary plane we
> would reset crtc_state->enable_fbc back to false even when FBC is
> enabled. That also wouldn't result in a bug due to the way the
> enable_fbc variable is checked, but, still, our code can be better
> than this.
>
> So I went for the solution that involves tracking enable_fbc in the
> primary plane state instead of the CRTC state. This way, if a commit
> doesn't involve the primary plane for the CRTC we won't be resetting
> enable_fbc back to false, so the variable will always reflect the
> reality. And this change also makes more sense since FBC is actually
> tied to the single plane and not the full pipe. As a bonus, we only
> iterate over the CRTCs instead of iterating over all planes.
>
> v2:
>
> But Ville pointed that this only works properly if we have a single
> commit with multiple CRTCs. If we have multiple parallel commits, each
> with its own CRTC, we'll end with enable_fbc set to true in more than
> one plane.
>
> So the solution was to rename enable_fbc to can_enable_fbc. If we just
> did the rename as the patch was, we'd end up with a single plane with
> can_enable_fbc on commits involving multiple CRTCs: we'd choose the
> best one, but we'd still end up with a variable that doesn't 100%
> reflect reality.
>
> So in the end I adopted Ville's suggestion of the fbc_score variable.
> And then, instead of checking the score at intel_fbc_choose_crtc()
> it should be possible to check for the score at intel_fbc_enable().
> This allows us to simplify intel_fbc_choose_crtc() to the point where
> it only needs to look at the given plane in order to assign its score
> instead of looking at every primary plane on the same commit.
>
> We still only set scores of 0 and 1 and we don't really do the
> score-checking loop.
>
> v3: Rebase.
>
> Cc: Ville Syrjälä
> Reported-by: Ville Syrjälä
> Signed-off-by: Paulo Zanoni
> ---
> drivers/gpu/drm/i915/intel_display.c | 3 +-
> drivers/gpu/drm/i915/intel_drv.h | 8 ++--
> drivers/gpu/drm/i915/intel_fbc.c | 85
>
> 3 files changed, 35 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index d8effd4..92527d6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14191,7 +14191,6 @@ static int intel_atomic_check(struct drm_device *dev,
> if (ret)
> return ret;
>
> - intel_fbc_choose_crtc(dev_priv, state);
> return calc_watermark_data(state);
> }
>
> @@ -14966,6 +14965,8 @@ intel_check_primary_plane(struct drm_plane *plane,
> return ret;
> }
>
> + intel_fbc_check_plane(state);
> +
We have another 'return 0' a little earlier in the function for the
"plane disable by the user" case. Presumably we'll want to update
the score in that case as well?
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 025e4c8..7430082 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -406,6 +406,9 @@ struct intel_plane_state {
>*/
> int scaler_id;
>
> + /* 0: not suitable for FBC, 1+: suitable for FBC, more is better. */
> + unsigned int fbc_score;
> +
> struct drm_intel_sprite_colorkey ckey;
> };
>
> @@ -652,8 +655,6 @@ struct intel_crtc_state {
>
> bool ips_enabled;
>
> - bool enable_fbc;
> -
> bool double_wide;
>
> int pbn;
> @@ -1535,8 +1536,7 @@ static inline void intel_fbdev_restore_mode(struct
> drm_device *dev)
> #endif
>
> /* intel_fbc.c */
> -void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv,
> -struct drm_atomic_state *state);
> +void intel_fbc_check_plane(struct intel_plane_state *plane_state);
> bool intel_fbc_is_active(struct drm_i915_private *dev_priv);
> void intel_fbc_pre_update(struct intel_crtc *crtc,
> struct intel_crtc_state *crtc_state,
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c
> b/drivers/gpu/drm/i915/intel_fbc.c
> index 26a81a9..ceda6f4 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -1040,68 +1040,32 @@ void intel_fbc_flush(struct drm_i915_private
> *dev_priv,
> }
>
> /**
> - * intel_fbc_choose_crtc - select a CRTC to enable FBC
[Intel-gfx] ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: enable FBC on gen9+ too
== Series Details == Series: series starting with [1/2] drm/i915: enable FBC on gen9+ too URL : https://patchwork.freedesktop.org/series/17176/ State : warning == Summary == Series 17176v1 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/17176/revisions/1/mbox/ Test kms_force_connector_basic: Subgroup force-connector-state: pass -> DMESG-WARN (fi-snb-2520m) Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-b: pass -> DMESG-WARN (fi-snb-2520m) fi-bdw-5557u total:246 pass:232 dwarn:0 dfail:0 fail:0 skip:14 fi-bsw-n3050 total:246 pass:207 dwarn:0 dfail:0 fail:0 skip:39 fi-bxt-j4205 total:246 pass:224 dwarn:0 dfail:0 fail:0 skip:22 fi-bxt-t5700 total:82 pass:69 dwarn:0 dfail:0 fail:0 skip:12 fi-byt-j1900 total:246 pass:219 dwarn:0 dfail:0 fail:0 skip:27 fi-byt-n2820 total:246 pass:215 dwarn:0 dfail:0 fail:0 skip:31 fi-hsw-4770 total:246 pass:227 dwarn:0 dfail:0 fail:0 skip:19 fi-hsw-4770r total:246 pass:227 dwarn:0 dfail:0 fail:0 skip:19 fi-ivb-3520m total:246 pass:225 dwarn:0 dfail:0 fail:0 skip:21 fi-ivb-3770 total:246 pass:225 dwarn:0 dfail:0 fail:0 skip:21 fi-kbl-7500u total:246 pass:225 dwarn:0 dfail:0 fail:0 skip:21 fi-skl-6260u total:246 pass:233 dwarn:0 dfail:0 fail:0 skip:13 fi-skl-6700hqtotal:246 pass:226 dwarn:0 dfail:0 fail:0 skip:20 fi-skl-6700k total:246 pass:222 dwarn:3 dfail:0 fail:0 skip:21 fi-skl-6770hqtotal:246 pass:233 dwarn:0 dfail:0 fail:0 skip:13 fi-snb-2520m total:246 pass:213 dwarn:2 dfail:0 fail:0 skip:31 fi-snb-2600 total:246 pass:214 dwarn:0 dfail:0 fail:0 skip:32 7165fdc7f0b0b536557fcd0a222d083a901be57c drm-tip: 2016y-12m-22d-19h-42m-25s UTC integration manifest 5fdaf94 drm/i915: rewrite FBC's atomic CRTC-choosing code be5982e drm/i915: enable FBC on gen9+ too == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3385/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 07/14] drm/i915: Start moving the cdclk stuff into a distinct state structure
On Fri, 2016-12-23 at 14:27 +0200, Ville Syrjälä wrote:
> On Fri, Dec 23, 2016 at 11:09:22AM +0200, Ander Conselvan De Oliveira wrote:
> >
> > On Thu, 2016-12-22 at 16:33 +0200, Ville Syrjälä wrote:
> > >
> > > On Thu, Dec 22, 2016 at 04:14:40PM +0200, Ander Conselvan De Oliveira
> > > wrote:
> > > >
> > > >
> > > > On Mon, 2016-12-19 at 19:28 +0200, ville.syrj...@linux.intel.com wrote:
> > > > >
> > > > >
> > > > > From: Ville Syrjälä
> > > > >
> > > > > Introduce intel_cdclk state which for now will track the cdclk
> > > > > frequency, the vco frequency and the reference frequency (not sure we
> > > > > want the last one, but I put it there anyway). We'll also make the
> > > > > .get_cdclk() function fill out this state structure rather than
> > > > > just returning the current cdclk frequency.
> > > > >
> > > > > One immediate benefit is that calling .get_cdclk() will no longer
> > > > > clobber state stored under dev_priv unless ex[plicitly told to do
> > > > Typo: ex[plicity
> > > >
> > > > >
> > > > >
> > > > > so. Previously it clobbered the vco and reference clocks stored
> > > > > there on some platforms.
> > > > >
> > > > > We'll expand the use of this structure to actually precomputing the
> > > > > state and whatnot later.
> > > > >
> > > > > v2: Constify intel_cdclk_state_compare()
> > > > >
> > > > > Signed-off-by: Ville Syrjälä
> > > > A couple more comments beloew, but either way,
> > > >
> > > > Reviewed-by: Ander Conselvan de Oliveira
> > > >
> > > > >
> > > > >
> > > > > ---
> > > > > drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
> > > > > drivers/gpu/drm/i915/i915_drv.h | 14 +-
> > > > > drivers/gpu/drm/i915/intel_audio.c | 2 +-
> > > > > drivers/gpu/drm/i915/intel_cdclk.c | 359 ++-
> > > > > -
> > > > > drivers/gpu/drm/i915/intel_display.c| 18 +-
> > > > > drivers/gpu/drm/i915/intel_dp.c | 2 +-
> > > > > drivers/gpu/drm/i915/intel_drv.h| 3 +
> > > > > drivers/gpu/drm/i915/intel_fbc.c| 2 +-
> > > > > drivers/gpu/drm/i915/intel_panel.c | 4 +-
> > > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 5 +-
> > > > > 10 files changed, 240 insertions(+), 171 deletions(-)
> > > > >
> > [...]
> >
> > @@ -629,12 +668,13 @@ static int skl_calc_cdclk(int max_pixclk, int vco)
> > >
> > > >
> > > > >
> > > > > }
> > > > >
> > > > > -static void skl_dpll0_update(struct drm_i915_private *dev_priv)
> > > > > +static void skl_dpll0_update(struct drm_i915_private *dev_priv,
> > > > > + struct intel_cdclk_state *cdclk_state)
> > > > > {
> > > > > u32 val;
> > > > >
> > > > > - dev_priv->cdclk_pll.ref = 24000;
> > > > > - dev_priv->cdclk_pll.vco = 0;
> > > > > + cdclk_state->ref = 24000;
> > > > > + cdclk_state->vco = 0;
> > > > >
> > > > > val = I915_READ(LCPLL1_CTL);
> > > > > if ((val & LCPLL_PLL_ENABLE) == 0)
> > > > > @@ -656,11 +696,11 @@ static void skl_dpll0_update(struct
> > > > > drm_i915_private *dev_priv)
> > > > > case DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1350,
> > > > > SKL_DPLL0):
> > > > > case DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1620,
> > > > > SKL_DPLL0):
> > > > > case DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_2700,
> > > > > SKL_DPLL0):
> > > > > - dev_priv->cdclk_pll.vco = 810;
> > > > > + cdclk_state->vco = 810;
> > > > > break;
> > > > > case DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1080,
> > > > > SKL_DPLL0):
> > > > > case DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_2160,
> > > > > SKL_DPLL0):
> > > > > - dev_priv->cdclk_pll.vco = 864;
> > > > > + cdclk_state->vco = 864;
> > > > > break;
> > > > > default:
> > > > > MISSING_CASE(val &
> > > > > DPLL_CTRL1_LINK_RATE_MASK(SKL_DPLL0));
> > > > This function is a bit funny now, since it sets up the cdclk state based
> > > > on
> > > > current pll0 programming, instead of looking at an atomic state.
> > > We should probably rename it to somehting more descriptive.
> > > skl_get_dpll0()
> > > maybe. Or maybe skl_dpll0_get_config to match the .get_config()
> > > nomenclature used for eg. encoders.
> > Hmm, I think the problem is that this have a mix of get_hw_state() and
> > get_config(). We actually have skl_ddi_dpll0_get_hw_state(), but it wouldn't
> > be
> > trivial to make that code reusable for the cdclk stuff.
> >
> > Maybe it should be two separate functions, one that gets the hardware state
> > and
> > another for choosing the vco.
> This function only exists to read the hw state.
Then get_hw_state() wouldn't be a bad name. The only difference between this and
the pll get_hw_state() hook is that the latter doesn't set a vco value, since
that is not directly programmed.
>
> >
> > Then
> >
> > vco = skl_cdclk_vco(skl_dpll0_get_hw_state());
> >
> > directly in skl_get_cdclk() ? I'm no
Re: [Intel-gfx] [PATCH] drm/915: Parsing the missed out DTD fields from the VBT
On Thu, 22 Dec 2016, Madhav Chauhan wrote:
> From: Vincente Tsou
>
> The upper bits of the vsync width, vsync offset and hsync width
> were not parsed from the VBT. Parse these fields in this patch.
>
> V2: Renamed lvds dvo timing structure members and code identation
> fix (Jani's review comments)
> V3: Corrected commit message, used "from the VBT"
>
> Signed-off-by: Vincente Tsou
> Signed-off-by: Madhav Chauhan
Pushed to dinq, thanks for the patch.
BR,
Jani.
> ---
> drivers/gpu/drm/i915/intel_bios.c | 8 +---
> drivers/gpu/drm/i915/intel_vbt_defs.h | 12 +++-
> 2 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.c
> b/drivers/gpu/drm/i915/intel_bios.c
> index a359def..f6d3755 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -114,16 +114,18 @@ static u32 get_blocksize(const void *block_data)
> panel_fixed_mode->hsync_start = panel_fixed_mode->hdisplay +
> ((dvo_timing->hsync_off_hi << 8) | dvo_timing->hsync_off_lo);
> panel_fixed_mode->hsync_end = panel_fixed_mode->hsync_start +
> - dvo_timing->hsync_pulse_width;
> + ((dvo_timing->hsync_pulse_width_hi << 8) |
> + dvo_timing->hsync_pulse_width_lo);
> panel_fixed_mode->htotal = panel_fixed_mode->hdisplay +
> ((dvo_timing->hblank_hi << 8) | dvo_timing->hblank_lo);
>
> panel_fixed_mode->vdisplay = (dvo_timing->vactive_hi << 8) |
> dvo_timing->vactive_lo;
> panel_fixed_mode->vsync_start = panel_fixed_mode->vdisplay +
> - dvo_timing->vsync_off;
> + ((dvo_timing->vsync_off_hi << 4) | dvo_timing->vsync_off_lo);
> panel_fixed_mode->vsync_end = panel_fixed_mode->vsync_start +
> - dvo_timing->vsync_pulse_width;
> + ((dvo_timing->vsync_pulse_width_hi << 4) |
> + dvo_timing->vsync_pulse_width_lo);
> panel_fixed_mode->vtotal = panel_fixed_mode->vdisplay +
> ((dvo_timing->vblank_hi << 8) | dvo_timing->vblank_lo);
> panel_fixed_mode->clock = dvo_timing->clock * 10;
> diff --git a/drivers/gpu/drm/i915/intel_vbt_defs.h
> b/drivers/gpu/drm/i915/intel_vbt_defs.h
> index 8886cab1..a92e776 100644
> --- a/drivers/gpu/drm/i915/intel_vbt_defs.h
> +++ b/drivers/gpu/drm/i915/intel_vbt_defs.h
> @@ -399,10 +399,12 @@ struct lvds_dvo_timing {
> u8 vblank_hi:4;
> u8 vactive_hi:4;
> u8 hsync_off_lo;
> - u8 hsync_pulse_width;
> - u8 vsync_pulse_width:4;
> - u8 vsync_off:4;
> - u8 rsvd0:6;
> + u8 hsync_pulse_width_lo;
> + u8 vsync_pulse_width_lo:4;
> + u8 vsync_off_lo:4;
> + u8 vsync_pulse_width_hi:2;
> + u8 vsync_off_hi:2;
> + u8 hsync_pulse_width_hi:2;
> u8 hsync_off_hi:2;
> u8 himage_lo;
> u8 vimage_lo;
> @@ -414,7 +416,7 @@ struct lvds_dvo_timing {
> u8 digital:2;
> u8 vsync_positive:1;
> u8 hsync_positive:1;
> - u8 rsvd2:1;
> + u8 non_interlaced:1;
> } __packed;
>
> struct lvds_pnp_id {
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: rewrite FBC's atomic CRTC-choosing code
Em Sex, 2016-12-23 às 14:43 +0200, Ville Syrjälä escreveu:
> On Fri, Dec 23, 2016 at 10:23:59AM -0200, Paulo Zanoni wrote:
> >
> > Ville pointed out that intel_fbc_choose_crtc() is iterating over
> > all
> > planes instead of just the primary planes. There are no real
> > consequences of this problem for HSW+, and for the other platforms
> > it
> > just means that in some obscure multi-screen cases we'll keep FBC
> > disabled when we could have enabled it. Still, iterating over all
> > planes doesn't seem to be the best thing to do.
> >
> > My initial idea was to just add a check for plane->type and be
> > done,
> > but then I realized that in commits not involving the primary plane
> > we
> > would reset crtc_state->enable_fbc back to false even when FBC is
> > enabled. That also wouldn't result in a bug due to the way the
> > enable_fbc variable is checked, but, still, our code can be better
> > than this.
> >
> > So I went for the solution that involves tracking enable_fbc in the
> > primary plane state instead of the CRTC state. This way, if a
> > commit
> > doesn't involve the primary plane for the CRTC we won't be
> > resetting
> > enable_fbc back to false, so the variable will always reflect the
> > reality. And this change also makes more sense since FBC is
> > actually
> > tied to the single plane and not the full pipe. As a bonus, we only
> > iterate over the CRTCs instead of iterating over all planes.
> >
> > v2:
> >
> > But Ville pointed that this only works properly if we have a single
> > commit with multiple CRTCs. If we have multiple parallel commits,
> > each
> > with its own CRTC, we'll end with enable_fbc set to true in more
> > than
> > one plane.
> >
> > So the solution was to rename enable_fbc to can_enable_fbc. If we
> > just
> > did the rename as the patch was, we'd end up with a single plane
> > with
> > can_enable_fbc on commits involving multiple CRTCs: we'd choose the
> > best one, but we'd still end up with a variable that doesn't 100%
> > reflect reality.
> >
> > So in the end I adopted Ville's suggestion of the fbc_score
> > variable.
> > And then, instead of checking the score at intel_fbc_choose_crtc()
> > it should be possible to check for the score at intel_fbc_enable().
> > This allows us to simplify intel_fbc_choose_crtc() to the point
> > where
> > it only needs to look at the given plane in order to assign its
> > score
> > instead of looking at every primary plane on the same commit.
> >
> > We still only set scores of 0 and 1 and we don't really do the
> > score-checking loop.
> >
> > v3: Rebase.
> >
> > Cc: Ville Syrjälä
> > Reported-by: Ville Syrjälä
> > Signed-off-by: Paulo Zanoni
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 3 +-
> > drivers/gpu/drm/i915/intel_drv.h | 8 ++--
> > drivers/gpu/drm/i915/intel_fbc.c | 85 --
> > --
> > 3 files changed, 35 insertions(+), 61 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index d8effd4..92527d6 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -14191,7 +14191,6 @@ static int intel_atomic_check(struct
> > drm_device *dev,
> > if (ret)
> > return ret;
> >
> > - intel_fbc_choose_crtc(dev_priv, state);
> > return calc_watermark_data(state);
> > }
> >
> > @@ -14966,6 +14965,8 @@ intel_check_primary_plane(struct drm_plane
> > *plane,
> > return ret;
> > }
> >
> > + intel_fbc_check_plane(state);
> > +
>
> We have another 'return 0' a little earlier in the function for the
> "plane disable by the user" case. Presumably we'll want to update
> the score in that case as well?
I always assumed the state was zero-allocated, not copied, and int his
case this wouldn't be a problem... Thanks for pointing this to me.
I'll move it up. Can't move up further since we need plane_state-
>visible to be set.
>
> >
> > return 0;
> > }
> >
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 025e4c8..7430082 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -406,6 +406,9 @@ struct intel_plane_state {
> > */
> > int scaler_id;
> >
> > + /* 0: not suitable for FBC, 1+: suitable for FBC, more is
> > better. */
> > + unsigned int fbc_score;
> > +
> > struct drm_intel_sprite_colorkey ckey;
> > };
> >
> > @@ -652,8 +655,6 @@ struct intel_crtc_state {
> >
> > bool ips_enabled;
> >
> > - bool enable_fbc;
> > -
> > bool double_wide;
> >
> > int pbn;
> > @@ -1535,8 +1536,7 @@ static inline void
> > intel_fbdev_restore_mode(struct drm_device *dev)
> > #endif
> >
> > /* intel_fbc.c */
> > -void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv,
> > - struct drm_atomic_state *state);
> > +void intel_fbc_check_plane(struct intel_plane_s
Re: [Intel-gfx] [PATCH 1/2] drm : adds Y-coordinate and Colorimetry Format
On Thu, 22 Dec 2016, vathsala nagaraju wrote: > PSR2 vsc revision number hb2( as per table 6-11)is updated to > 4 or 5 based on Y cordinate and Colorimetry Format as below > 04h = 3D stereo + PSR/PSR2 + Y-coordinate. > 05h = -3D stereo- + PSR/PSR2 + Y-coordinate + Pixel Encoding/Colorimetry > Format indication. A DP Source device is allowed to indicate the pixel > encoding/colorimetry format to the DP Sink device with VSC SDP only when > the DP Sink device supports it ( > i.e.,VSC_SDP_EXTENSION_FOR_COLORIMETRY_SUPPORTED bit in the > DPRX_FEATURE_ENUMERATION_LIST register (DPCD Address 02210h, bit 3; > is set to 1). > > v2: (Jani) > - Change DP_PSR_Y_COORDINATE to DP_PSR2_SU_Y_COORDINATE_REQUIRED. > - Add DP_PSR2_SU_GRANULARITY_REQUIRED. > - Change DPRX_FEATURE_ENUMERATION_LIST to DP_DPRX. > - Add GTC_CAP and AV_SYNC_CAP, other bits in DPRX_FEATURE_ENUMERATION_LIST. > > Cc: Rodrigo Vivi > Cc: Jim Bride > Signed-off-by: Vathsala Nagaraju > --- > include/drm/drm_dp_helper.h | 8 +++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h > index 55bbeb0..ee2a649d 100644 > --- a/include/drm/drm_dp_helper.h > +++ b/include/drm/drm_dp_helper.h > @@ -194,7 +194,8 @@ > # define DP_PSR_SETUP_TIME_0(6 << 1) > # define DP_PSR_SETUP_TIME_MASK (7 << 1) > # define DP_PSR_SETUP_TIME_SHIFT1 > - > +# define DP_PSR2_SU_Y_COORDINATE_REQUIRED (1 << 4) /* eDP 1.4a */ > +# define DP_PSR2_SU_GRANULARITY_REQUIRED(1 << 5) /* eDP 1.4b */ > /* > * 0x80-0x8f describe downstream port capabilities, but there are two layouts > * based on whether DP_DETAILED_CAP_INFO_AVAILABLE was set. If it was not, > @@ -568,6 +569,11 @@ > #define DP_RECEIVER_ALPM_STATUS 0x200b /* eDP 1.4 */ > # define DP_ALPM_LOCK_TIMEOUT_ERROR (1 << 0) > > +#define DP_DPRX_FEATURE_ENUMERATION_LIST0x2210 > +# define DP_GTC_CAP (1 << 0) > +# define DP_AV_SYNC_CAP (1 << 2) > +# define DP_VSC_SDP_EXT_FOR_COLORIMETRY_SUPPORTED(1 << 3) The spec continues with bits 4 to 7 in the next page... > + > /* DP 1.2 Sideband message defines */ > /* peer device type - DP 1.2a Table 2-92 */ > #define DP_PEER_DEVICE_NONE 0x0 -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 06/18] drm/i915/dsi: Move disable pll call outside of clear_device_ready()
On Thu, 01 Dec 2016, Hans de Goede wrote:
> On enable intel_dsi_enable() directly calls intel_enable_dsi_pll(),
> make intel_dsi_disable() also directly call intel_disable_dsi_pll(),
> rather then hiding the call in intel_dsi_clear_device_ready(),
> no functional changes.
>
> Signed-off-by: Hans de Goede
Pushed this one to drm-intel-next-queued, thanks for the patch.
BR,
Jani.
> ---
> drivers/gpu/drm/i915/intel_dsi.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> b/drivers/gpu/drm/i915/intel_dsi.c
> index 967bae9..cb15c0a 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -642,8 +642,6 @@ static void intel_dsi_clear_device_ready(struct
> intel_encoder *encoder)
> I915_WRITE(MIPI_DEVICE_READY(port), 0x00);
> usleep_range(2000, 2500);
> }
> -
> - intel_disable_dsi_pll(encoder);
> }
>
> static void intel_dsi_post_disable(struct intel_encoder *encoder,
> @@ -674,6 +672,8 @@ static void intel_dsi_post_disable(struct intel_encoder
> *encoder,
>
> intel_dsi_clear_device_ready(encoder);
>
> + intel_disable_dsi_pll(encoder);
> +
> if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> u32 val;
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: rewrite FBC's atomic CRTC-choosing code
On Fri, Dec 23, 2016 at 11:23:56AM -0200, Paulo Zanoni wrote:
> Em Sex, 2016-12-23 às 14:43 +0200, Ville Syrjälä escreveu:
> > On Fri, Dec 23, 2016 at 10:23:59AM -0200, Paulo Zanoni wrote:
> > >
> > > Ville pointed out that intel_fbc_choose_crtc() is iterating over
> > > all
> > > planes instead of just the primary planes. There are no real
> > > consequences of this problem for HSW+, and for the other platforms
> > > it
> > > just means that in some obscure multi-screen cases we'll keep FBC
> > > disabled when we could have enabled it. Still, iterating over all
> > > planes doesn't seem to be the best thing to do.
> > >
> > > My initial idea was to just add a check for plane->type and be
> > > done,
> > > but then I realized that in commits not involving the primary plane
> > > we
> > > would reset crtc_state->enable_fbc back to false even when FBC is
> > > enabled. That also wouldn't result in a bug due to the way the
> > > enable_fbc variable is checked, but, still, our code can be better
> > > than this.
> > >
> > > So I went for the solution that involves tracking enable_fbc in the
> > > primary plane state instead of the CRTC state. This way, if a
> > > commit
> > > doesn't involve the primary plane for the CRTC we won't be
> > > resetting
> > > enable_fbc back to false, so the variable will always reflect the
> > > reality. And this change also makes more sense since FBC is
> > > actually
> > > tied to the single plane and not the full pipe. As a bonus, we only
> > > iterate over the CRTCs instead of iterating over all planes.
> > >
> > > v2:
> > >
> > > But Ville pointed that this only works properly if we have a single
> > > commit with multiple CRTCs. If we have multiple parallel commits,
> > > each
> > > with its own CRTC, we'll end with enable_fbc set to true in more
> > > than
> > > one plane.
> > >
> > > So the solution was to rename enable_fbc to can_enable_fbc. If we
> > > just
> > > did the rename as the patch was, we'd end up with a single plane
> > > with
> > > can_enable_fbc on commits involving multiple CRTCs: we'd choose the
> > > best one, but we'd still end up with a variable that doesn't 100%
> > > reflect reality.
> > >
> > > So in the end I adopted Ville's suggestion of the fbc_score
> > > variable.
> > > And then, instead of checking the score at intel_fbc_choose_crtc()
> > > it should be possible to check for the score at intel_fbc_enable().
> > > This allows us to simplify intel_fbc_choose_crtc() to the point
> > > where
> > > it only needs to look at the given plane in order to assign its
> > > score
> > > instead of looking at every primary plane on the same commit.
> > >
> > > We still only set scores of 0 and 1 and we don't really do the
> > > score-checking loop.
> > >
> > > v3: Rebase.
> > >
> > > Cc: Ville Syrjälä
> > > Reported-by: Ville Syrjälä
> > > Signed-off-by: Paulo Zanoni
> > > ---
> > > drivers/gpu/drm/i915/intel_display.c | 3 +-
> > > drivers/gpu/drm/i915/intel_drv.h | 8 ++--
> > > drivers/gpu/drm/i915/intel_fbc.c | 85 --
> > > --
> > > 3 files changed, 35 insertions(+), 61 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index d8effd4..92527d6 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -14191,7 +14191,6 @@ static int intel_atomic_check(struct
> > > drm_device *dev,
> > > if (ret)
> > > return ret;
> > >
> > > - intel_fbc_choose_crtc(dev_priv, state);
> > > return calc_watermark_data(state);
> > > }
> > >
> > > @@ -14966,6 +14965,8 @@ intel_check_primary_plane(struct drm_plane
> > > *plane,
> > > return ret;
> > > }
> > >
> > > + intel_fbc_check_plane(state);
> > > +
> >
> > We have another 'return 0' a little earlier in the function for the
> > "plane disable by the user" case. Presumably we'll want to update
> > the score in that case as well?
>
> I always assumed the state was zero-allocated, not copied, and int his
> case this wouldn't be a problem... Thanks for pointing this to me.
>
> I'll move it up. Can't move up further since we need plane_state-
> >visible to be set.
Maybe we just want to clear the score to 0 when visible==false?
>
>
> >
> > >
> > > return 0;
> > > }
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > b/drivers/gpu/drm/i915/intel_drv.h
> > > index 025e4c8..7430082 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -406,6 +406,9 @@ struct intel_plane_state {
> > > */
> > > int scaler_id;
> > >
> > > + /* 0: not suitable for FBC, 1+: suitable for FBC, more is
> > > better. */
> > > + unsigned int fbc_score;
> > > +
> > > struct drm_intel_sprite_colorkey ckey;
> > > };
> > >
> > > @@ -652,8 +655,6 @@ struct intel_crtc_state {
> > >
> > > bool ips_enabled;
> > >
> > > - bool enable_fbc;
> > > -
>
Re: [Intel-gfx] [PATCH v2 10/14] drm/i915: Pass the cdclk state to the set_cdclk() functions
On Mon, 2016-12-19 at 19:28 +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Rather than passing all the different parameters (cdclk,vco so
> far) sparately to the set_cdclk() functions, just pass the
> entire cdclk state.
>
> v2: Deal with churn
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_cdclk.c | 79
> +++---
> 1 file changed, 49 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c
> b/drivers/gpu/drm/i915/intel_cdclk.c
> index 2e998fa087a9..11a0f3e122c3 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -433,8 +433,10 @@ static void vlv_program_pfi_credits(struct
> drm_i915_private *dev_priv)
> WARN_ON(I915_READ(GCI_CONTROL) & PFI_CREDIT_RESEND);
> }
>
> -static void vlv_set_cdclk(struct drm_i915_private *dev_priv, int cdclk)
> +static void vlv_set_cdclk(struct drm_i915_private *dev_priv,
> + const struct intel_cdclk_state *cdclk_state)
> {
> + int cdclk = cdclk_state->cdclk;
> u32 val, cmd;
>
> if (cdclk >= 32) /* jump to highest voltage for 400MHz too */
> @@ -495,8 +497,10 @@ static void vlv_set_cdclk(struct drm_i915_private
> *dev_priv, int cdclk)
> intel_update_cdclk(dev_priv);
> }
>
> -static void chv_set_cdclk(struct drm_i915_private *dev_priv, int cdclk)
> +static void chv_set_cdclk(struct drm_i915_private *dev_priv,
> + const struct intel_cdclk_state *cdclk_state)
> {
> + int cdclk = cdclk_state->cdclk;
> u32 val, cmd;
>
> switch (cdclk) {
> @@ -564,8 +568,10 @@ static void bdw_get_cdclk(struct drm_i915_private
> *dev_priv,
> cdclk_state->cdclk = 675000;
> }
>
> -static void bdw_set_cdclk(struct drm_i915_private *dev_priv, int cdclk)
> +static void bdw_set_cdclk(struct drm_i915_private *dev_priv,
> + const struct intel_cdclk_state *cdclk_state)
> {
> + int cdclk = cdclk_state->cdclk;
> uint32_t val, data;
> int ret;
>
> @@ -836,8 +842,10 @@ static void skl_dpll0_disable(struct drm_i915_private
> *dev_priv)
> }
>
> static void skl_set_cdclk(struct drm_i915_private *dev_priv,
> - int cdclk, int vco)
> + const struct intel_cdclk_state *cdclk_state)
> {
> + int cdclk = cdclk_state->cdclk;
> + int vco = cdclk_state->vco;
> u32 freq_select, pcu_ack;
> int ret;
>
> @@ -942,12 +950,17 @@ static void skl_sanitize_cdclk(struct drm_i915_private
> *dev_priv)
>
> void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
> {
> - skl_set_cdclk(dev_priv, dev_priv->cdclk.hw.ref, 0);
> + struct intel_cdclk_state cdclk_state = dev_priv->cdclk.hw;
> +
> + cdclk_state.cdclk = cdclk_state.ref;
> + cdclk_state.vco = 0;
> +
> + skl_set_cdclk(dev_priv, &cdclk_state);
> }
>
> void skl_init_cdclk(struct drm_i915_private *dev_priv)
> {
> - int cdclk, vco;
> + struct intel_cdclk_state cdclk_state;
>
> skl_sanitize_cdclk(dev_priv);
>
> @@ -963,12 +976,14 @@ void skl_init_cdclk(struct drm_i915_private *dev_priv)
> return;
> }
>
> - vco = dev_priv->skl_preferred_vco_freq;
> - if (vco == 0)
> - vco = 810;
> - cdclk = skl_calc_cdclk(0, vco);
> + cdclk_state = dev_priv->cdclk.hw;
We need this assignments to preserve cdclk.ref?
> +
> + cdclk_state.vco = dev_priv->skl_preferred_vco_freq;
> + if (cdclk_state.vco == 0)
> + cdclk_state.vco = 810;
> + cdclk_state.cdclk = skl_calc_cdclk(0, cdclk_state.vco);
>
> - skl_set_cdclk(dev_priv, cdclk, vco);
> + skl_set_cdclk(dev_priv, &cdclk_state);
> }
>
> static int bxt_calc_cdclk(int max_pixclk)
> @@ -1132,8 +1147,10 @@ static void bxt_de_pll_enable(struct drm_i915_private
> *dev_priv, int vco)
> }
>
> static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
> - int cdclk, int vco)
> + const struct intel_cdclk_state *cdclk_state)
> {
> + int cdclk = cdclk_state->cdclk;
> + int vco = cdclk_state->vco;
> u32 val, divider;
> int ret;
>
> @@ -1259,7 +1276,7 @@ static void bxt_sanitize_cdclk(struct drm_i915_private
> *dev_priv)
>
> void bxt_init_cdclk(struct drm_i915_private *dev_priv)
> {
> - int cdclk, vco;
> + struct intel_cdclk_state cdclk_state;
>
> bxt_sanitize_cdclk(dev_priv);
>
> @@ -1267,25 +1284,33 @@ void bxt_init_cdclk(struct drm_i915_private *dev_priv)
> dev_priv->cdclk.hw.vco != 0)
> return;
>
> + cdclk_state = dev_priv->cdclk.hw;
> +
> /*
> * FIXME:
> * - The initial CDCLK needs to be read from VBT.
> * Need to make this change after VBT has changes for BXT.
> */
> if (IS_GEMINILAKE(dev_priv)) {
> - cdclk = glk_calc_cdclk(0);
> - vco = glk_de_pll_vco(dev
Re: [Intel-gfx] [PATCH 11/14] drm/i915: Move PFI credit reprogramming into vlv/chv_set_cdclk()
On Mon, 2016-12-19 at 19:28 +0200, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > Move the vlv_program_pfi_credits() into vlv_set_cdclk() and > chv_set_cdclk() so that we can neuter vlv_modeset_commit_cdclk(). > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/intel_cdclk.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c > b/drivers/gpu/drm/i915/intel_cdclk.c > index 11a0f3e122c3..fe7a9e3a4f29 100644 > --- a/drivers/gpu/drm/i915/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > @@ -494,6 +494,8 @@ static void vlv_set_cdclk(struct drm_i915_private > *dev_priv, > > mutex_unlock(&dev_priv->sb_lock); > > + vlv_program_pfi_credits(dev_priv); > + > intel_update_cdclk(dev_priv); > } > > @@ -533,6 +535,8 @@ static void chv_set_cdclk(struct drm_i915_private > *dev_priv, > } > mutex_unlock(&dev_priv->rps.hw_lock); > > + vlv_program_pfi_credits(dev_priv); > + > intel_update_cdclk(dev_priv); Reviewed-by: Ander Conselvan de Oliveira Can the vlv/chv register write in intel_update_cdclk() be moved to the set_cdclk() funcs too? > } > > @@ -1433,7 +1437,6 @@ static void vlv_modeset_commit_cdclk(struct > drm_atomic_state *old_state) > else > vlv_set_cdclk(dev_priv, &dev_priv->cdclk.actual); > > - vlv_program_pfi_credits(dev_priv); > > intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_A); > } ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 12/14] drm/i915: Nuke the VLV/CHV PFI programming power domain workaround
On Mon, 2016-12-19 at 19:28 +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> The hack to grab the pipe A power domain around VLV/CHV cdclk
> programming has surely outlived its usefulness. We should be
> hold sufficient power domains during any modeset, so let's just
hold/be holding?
Reviewed-by: Ander Conselvan de Oliveira
> nuke this hack.
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_cdclk.c | 14 --
> 1 file changed, 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c
> b/drivers/gpu/drm/i915/intel_cdclk.c
> index fe7a9e3a4f29..bba077c4bd56 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -1421,24 +1421,10 @@ static void vlv_modeset_commit_cdclk(struct
> drm_atomic_state *old_state)
> {
> struct drm_i915_private *dev_priv = to_i915(old_state->dev);
>
> - /*
> - * FIXME: We can end up here with all power domains off, yet
> - * with a CDCLK frequency other than the minimum. To account
> - * for this take the PIPE-A power domain, which covers the HW
> - * blocks needed for the following programming. This can be
> - * removed once it's guaranteed that we get here either with
> - * the minimum CDCLK set, or the required power domains
> - * enabled.
> - */
> - intel_display_power_get(dev_priv, POWER_DOMAIN_PIPE_A);
> -
> if (IS_CHERRYVIEW(dev_priv))
> chv_set_cdclk(dev_priv, &dev_priv->cdclk.actual);
> else
> vlv_set_cdclk(dev_priv, &dev_priv->cdclk.actual);
> -
> -
> - intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_A);
> }
>
> static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [GLK MIPI DSI V2 1/9] drm/i915/glk: Add new bit fields in MIPI CTRL register
On Thu, 15 Dec 2016, Madhav Chauhan wrote:
> From: Deepak M
>
> v2: Addressed Jani's Review comments (renamed bit field macros)
>
> Signed-off-by: Deepak M
> Signed-off-by: Madhav Chauhan
Pushed to drm-intel-next-queued, thanks for the patch.
BR,
Jani.
> ---
> drivers/gpu/drm/i915/i915_reg.h | 15 +++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 90685d2..8e47b59 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -8672,6 +8672,21 @@ enum {
> #define BXT_PIPE_SELECT_SHIFT 7
> #define BXT_PIPE_SELECT_MASK(7 << 7)
> #define BXT_PIPE_SELECT(pipe) ((pipe) << 7)
> +#define GLK_PHY_STATUS_PORT_READY (1 << 31) /* RO */
> +#define GLK_ULPS_NOT_ACTIVE (1 << 30) /* RO */
> +#define GLK_MIPIIO_RESET_RELEASED (1 << 28)
> +#define GLK_CLOCK_LANE_STOP_STATE (1 << 27) /* RO */
> +#define GLK_DATA_LANE_STOP_STATE(1 << 26) /* RO */
> +#define GLK_LP_WAKE (1 << 22)
> +#define GLK_LP11_LOW_PWR_MODE (1 << 21)
> +#define GLK_LP00_LOW_PWR_MODE (1 << 20)
> +#define GLK_FIREWALL_ENABLE (1 << 16)
> +#define BXT_PIXEL_OVERLAP_CNT_MASK (0xf << 10)
> +#define BXT_PIXEL_OVERLAP_CNT_SHIFT 10
> +#define BXT_DSC_ENABLE (1 << 3)
> +#define BXT_RGB_FLIP(1 << 2)
> +#define GLK_MIPIIO_PORT_POWERED (1 << 1) /* RO */
> +#define GLK_MIPIIO_ENABLE (1 << 0)
>
> #define _MIPIA_DATA_ADDRESS (dev_priv->mipi_mmio_base + 0xb108)
> #define _MIPIC_DATA_ADDRESS (dev_priv->mipi_mmio_base + 0xb908)
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [GLK MIPI DSI V2 2/9] drm/i915/glk: Program new MIPI DSI PHY registers for GLK
On Thu, 15 Dec 2016, Madhav Chauhan wrote:
> From: Deepak M
>
> Program the clk lane and tlpx time count registers
> to configure DSI PHY.
>
> v2: Addressed Jani's Review comments(renamed bit field macros)
>
> Signed-off-by: Deepak M
> Signed-off-by: Madhav Chauhan
> ---
> drivers/gpu/drm/i915/i915_reg.h | 18 ++
> drivers/gpu/drm/i915/intel_dsi.c | 15 +++
> 2 files changed, 33 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8e47b59..03858f9 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -8550,6 +8550,24 @@ enum {
> #define LP_BYTECLK_SHIFT0
> #define LP_BYTECLK_MASK (0x << 0)
>
> +#define _MIPIA_TLPX_TIME_COUNT (dev_priv->mipi_mmio_base +
> 0xb0a4)
> +#define _MIPIC_TLPX_TIME_COUNT (dev_priv->mipi_mmio_base +
> 0xb8a4)
> +#define MIPI_TLPX_TIME_COUNT(port)_MMIO_MIPI(port,
> _MIPIA_TLPX_TIME_COUNT, _MIPIC_TLPX_TIME_COUNT)
> +#define GLK_DPHY_TLPX_TIME_CNT_SHIFT0
> +#define GLK_DPHY_TLPX_TIME_CNT_MASK (0xff << 0)
> +
> +#define _MIPIA_CLK_LANE_TIMING (dev_priv->mipi_mmio_base +
> 0xb098)
> +#define _MIPIC_CLK_LANE_TIMING (dev_priv->mipi_mmio_base +
> 0xb898)
> +#define MIPI_CLK_LANE_TIMING(port)_MMIO_MIPI(port,
> _MIPIA_CLK_LANE_TIMING, _MIPIC_CLK_LANE_TIMING)
> +#define GLK_MIPI_CLK_LANE_HS_PREP_SHIFT 0
> +#define GLK_MIPI_CLK_LANE_HS_PREP_MASK (0xff << 0)
> +#define GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT 8
> +#define GLK_MIPI_CLK_LANE_HS_ZERO_MASK (0xff00 << 0)
0xff << 8
> +#define GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT16
> +#define GLK_MIPI_CLK_LANE_HS_TRAIL_MASK (0xff << 0)
0xff << 16
> +#define GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT 24
> +#define GLK_MIPI_CLK_LANE_HS_EXIT_MASK (0xff00 <<
> 0)
0xff << 24
> +
> /* bits 31:0 */
> #define _MIPIA_LP_GEN_DATA (dev_priv->mipi_mmio_base + 0xb064)
> #define _MIPIC_LP_GEN_DATA (dev_priv->mipi_mmio_base + 0xb864)
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> b/drivers/gpu/drm/i915/intel_dsi.c
> index 6b63355..b78c686 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -1123,6 +1123,7 @@ static void intel_dsi_prepare(struct intel_encoder
> *intel_encoder,
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
> struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
> + struct mipi_config *mipi_config = dev_priv->vbt.dsi.config;
> const struct drm_display_mode *adjusted_mode =
> &pipe_config->base.adjusted_mode;
> enum port port;
> unsigned int bpp =
> mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
> @@ -1278,6 +1279,20 @@ static void intel_dsi_prepare(struct intel_encoder
> *intel_encoder,
>*/
> I915_WRITE(MIPI_LP_BYTECLK(port), intel_dsi->lp_byte_clk);
>
> + if (IS_GEMINILAKE(dev_priv)) {
> + I915_WRITE(MIPI_TLPX_TIME_COUNT(port),
> + intel_dsi->lp_byte_clk);
> + val = ((mipi_config->ths_prepare <<
> + GLK_MIPI_CLK_LANE_HS_PREP_SHIFT) |
> + (mipi_config->ths_prepare_hszero <<
> + GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT) |
> + (mipi_config->ths_trail <<
> + GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT) |
> + (mipi_config->ths_exit <<
> + GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT));
> + I915_WRITE(MIPI_CLK_LANE_TIMING(port), val);
> + }
Please fix this as Ville suggested, i.e. don't look at mipi_config
directly in intel_dsi.c. See what gets done with dphy_reg.
We may change this later, but for now I think this is the right thing to
do.
BR,
Jani.
> +
> /* the bw essential for transmitting 16 long packets containing
>* 252 bytes meant for dcs write memory command is programmed in
>* this register in terms of byte clocks. based on dsi transfer
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v2] igt_core: add igt_constructor
On Wed, Dec 21, 2016 at 03:50:18PM -0500, Lyude wrote: > This is a simple macro for executing a block of code at the beginning of > intel-gpu-tools, before any tests have been ran. Useful for > initialization of global resources used in IGT libraries. > > Signed-off-by: Lyude > > Changes since v1: > - Add the line number into the name of the constructor function so that >multiple constructors may be used per-file. Works for me, Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 13/14] drm/i915: Replace the .modeset_commit_cdclk() hook with a more direct .set_cdclk() hook
On Mon, 2016-12-19 at 19:28 +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> With the cdclk state, all the .modeset_commit_cdclk() hooks are
> now pointless wrappers. Let's replace them with just a .set_cdclk()
> function pointer. However let's wrap that in a small helper that
> does the state comparison and prints a unified debug message across
> all platforms. We didn't even have the debug print on all platforms
> previously. This reduces the clutter in intel_atomic_commit_tail() a
> little bit.
>
> v2: Wrap .set_cdclk() in intel_set_cdclk()
>
> Signed-off-by: Ville Syrjälä
Nice cleanup!
Reviewed-by: Ander Conselvan de Oliveira
> ---
> drivers/gpu/drm/i915/i915_drv.h | 3 +-
> drivers/gpu/drm/i915/intel_cdclk.c | 71 +
> ---
> drivers/gpu/drm/i915/intel_display.c | 5 +--
> drivers/gpu/drm/i915/intel_drv.h | 2 +
> 4 files changed, 30 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b5a8d0f4cfbd..faf753570a9a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -614,6 +614,8 @@ struct intel_cdclk_state;
> struct drm_i915_display_funcs {
> void (*get_cdclk)(struct drm_i915_private *dev_priv,
> struct intel_cdclk_state *cdclk_state);
> + void (*set_cdclk)(struct drm_i915_private *dev_priv,
> + const struct intel_cdclk_state *cdclk_state);
> int (*get_fifo_size)(struct drm_i915_private *dev_priv, int plane);
> int (*compute_pipe_wm)(struct intel_crtc_state *cstate);
> int (*compute_intermediate_wm)(struct drm_device *dev,
> @@ -628,7 +630,6 @@ struct drm_i915_display_funcs {
> int (*compute_global_watermarks)(struct drm_atomic_state *state);
> void (*update_wm)(struct intel_crtc *crtc);
> int (*modeset_calc_cdclk)(struct drm_atomic_state *state);
> - void (*modeset_commit_cdclk)(struct drm_atomic_state *state);
> /* Returns the active state of the crtc, and if the crtc is active,
> * fills out the pipe-config with the hw state. */
> bool (*get_pipe_config)(struct intel_crtc *,
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c
> b/drivers/gpu/drm/i915/intel_cdclk.c
> index bba077c4bd56..c2d995169528 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -855,9 +855,6 @@ static void skl_set_cdclk(struct drm_i915_private
> *dev_priv,
>
> WARN_ON((cdclk == 24000) != (vco == 0));
>
> - DRM_DEBUG_DRIVER("Changing CDCLK to %d kHz (VCO %d kHz)\n",
> - cdclk, vco);
> -
> mutex_lock(&dev_priv->rps.hw_lock);
> ret = skl_pcode_request(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> SKL_CDCLK_PREPARE_FOR_CHANGE,
> @@ -1158,9 +1155,6 @@ static void bxt_set_cdclk(struct drm_i915_private
> *dev_priv,
> u32 val, divider;
> int ret;
>
> - DRM_DEBUG_DRIVER("Changing CDCLK to %d kHz (VCO %d kHz)\n",
> - cdclk, vco);
> -
> /* cdclk = vco / 2 / div{1,1.5,2,4} */
> switch (DIV_ROUND_CLOSEST(vco, cdclk)) {
> case 8:
> @@ -1323,6 +1317,22 @@ bool intel_cdclk_state_compare(const struct
> intel_cdclk_state *a,
> return memcmp(a, b, sizeof(*a)) == 0;
> }
>
> +void intel_set_cdclk(struct drm_i915_private *dev_priv,
> + const struct intel_cdclk_state *cdclk_state)
> +{
> + if (intel_cdclk_state_compare(&dev_priv->cdclk.hw, cdclk_state))
> + return;
> +
> + if (WARN_ON_ONCE(!dev_priv->display.set_cdclk))
> + return;
> +
> + DRM_DEBUG_DRIVER("Changing CDCLK to %d kHz, VCO %d kHz, ref %d
> kHz\n",
> + cdclk_state->cdclk, cdclk_state->vco,
> + cdclk_state->ref);
> +
> + dev_priv->display.set_cdclk(dev_priv, cdclk_state);
> +}
> +
> static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state
> *crtc_state,
> int pixel_rate)
> {
> @@ -1417,16 +1427,6 @@ static int vlv_modeset_calc_cdclk(struct
> drm_atomic_state *state)
> return 0;
> }
>
> -static void vlv_modeset_commit_cdclk(struct drm_atomic_state *old_state)
> -{
> - struct drm_i915_private *dev_priv = to_i915(old_state->dev);
> -
> - if (IS_CHERRYVIEW(dev_priv))
> - chv_set_cdclk(dev_priv, &dev_priv->cdclk.actual);
> - else
> - vlv_set_cdclk(dev_priv, &dev_priv->cdclk.actual);
> -}
> -
> static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
> {
> struct drm_i915_private *dev_priv = to_i915(state->dev);
> @@ -1460,13 +1460,6 @@ static int bdw_modeset_calc_cdclk(struct
> drm_atomic_state *state)
> return 0;
> }
>
> -static void bdw_modeset_commit_cdclk(struct drm_atomic_state *old_state)
> -{
> - struct drm_i915_private *dev_priv = to_i915(old_state->dev);
> -
> - bdw_set_cdclk(dev_priv, &dev_
Re: [Intel-gfx] [PATCH 14/14] drm/i915: Move ilk_pipe_pixel_rate() to intel_display.c
On Mon, 2016-12-19 at 19:28 +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Move ilk_pipe_pixel_rate() next to its only caller
> (intel_crtc_compute_pixel_rate()).
Assuming a rebased patch 1,
Reviewed-by: Ander Conselvan de Oliveira
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_display.c | 35 +++
> drivers/gpu/drm/i915/intel_drv.h | 1 -
> drivers/gpu/drm/i915/intel_pm.c | 33 -
> 3 files changed, 35 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 409537011b28..5e18a598117c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6229,6 +6229,41 @@ static bool intel_crtc_supports_double_wide(const
> struct intel_crtc *crtc)
> (crtc->pipe == PIPE_A || IS_I915G(dev_priv));
> }
>
> +static uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state
> *pipe_config)
> +{
> + uint32_t pixel_rate;
> +
> + pixel_rate = pipe_config->base.adjusted_mode.crtc_clock;
> +
> + /*
> + * We only use IF-ID interlacing. If we ever use
> + * PF-ID we'll need to adjust the pixel_rate here.
> + */
> +
> + if (pipe_config->pch_pfit.enabled) {
> + uint64_t pipe_w, pipe_h, pfit_w, pfit_h;
> + uint32_t pfit_size = pipe_config->pch_pfit.size;
> +
> + pipe_w = pipe_config->pipe_src_w;
> + pipe_h = pipe_config->pipe_src_h;
> +
> + pfit_w = (pfit_size >> 16) & 0x;
> + pfit_h = pfit_size & 0x;
> + if (pipe_w < pfit_w)
> + pipe_w = pfit_w;
> + if (pipe_h < pfit_h)
> + pipe_h = pfit_h;
> +
> + if (WARN_ON(!pfit_w || !pfit_h))
> + return pixel_rate;
> +
> + pixel_rate = div_u64((uint64_t) pixel_rate * pipe_w * pipe_h,
> + pfit_w * pfit_h);
> + }
> +
> + return pixel_rate;
> +}
> +
> static void intel_crtc_compute_pixel_rate(struct intel_crtc_state
> *crtc_state)
> {
> struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc-
> >dev);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 40074827f01d..4fd03b73d863 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1791,7 +1791,6 @@ bool skl_wm_level_equals(const struct skl_wm_level *l1,
> bool skl_ddb_allocation_overlaps(const struct skl_ddb_entry **entries,
> const struct skl_ddb_entry *ddb,
> int ignore);
> -uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config);
> bool ilk_disable_lp_wm(struct drm_device *dev);
> int sanitize_rc6_option(struct drm_i915_private *dev_priv, int enable_rc6);
> static inline int intel_enable_rc6(void)
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index fe522ec21502..cd81b51291d6 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1701,39 +1701,6 @@ static void i845_update_wm(struct intel_crtc
> *unused_crtc)
> I915_WRITE(FW_BLC, fwater_lo);
> }
>
> -uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config)
> -{
> - uint32_t pixel_rate;
> -
> - pixel_rate = pipe_config->base.adjusted_mode.crtc_clock;
> -
> - /* We only use IF-ID interlacing. If we ever use PF-ID we'll need to
> - * adjust the pixel_rate here. */
> -
> - if (pipe_config->pch_pfit.enabled) {
> - uint64_t pipe_w, pipe_h, pfit_w, pfit_h;
> - uint32_t pfit_size = pipe_config->pch_pfit.size;
> -
> - pipe_w = pipe_config->pipe_src_w;
> - pipe_h = pipe_config->pipe_src_h;
> -
> - pfit_w = (pfit_size >> 16) & 0x;
> - pfit_h = pfit_size & 0x;
> - if (pipe_w < pfit_w)
> - pipe_w = pfit_w;
> - if (pipe_h < pfit_h)
> - pipe_h = pfit_h;
> -
> - if (WARN_ON(!pfit_w || !pfit_h))
> - return pixel_rate;
> -
> - pixel_rate = div_u64((uint64_t) pixel_rate * pipe_w * pipe_h,
> - pfit_w * pfit_h);
> - }
> -
> - return pixel_rate;
> -}
> -
> /* latency must be in 0.1us units. */
> static uint32_t ilk_wm_method1(uint32_t pixel_rate, uint8_t cpp, uint32_t
> latency)
> {
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/14] drm/i915: Move PFI credit reprogramming into vlv/chv_set_cdclk()
On Fri, Dec 23, 2016 at 03:49:02PM +0200, Ander Conselvan De Oliveira wrote: > On Mon, 2016-12-19 at 19:28 +0200, ville.syrj...@linux.intel.com wrote: > > From: Ville Syrjälä > > > > Move the vlv_program_pfi_credits() into vlv_set_cdclk() and > > chv_set_cdclk() so that we can neuter vlv_modeset_commit_cdclk(). > > > > Signed-off-by: Ville Syrjälä > > --- > > drivers/gpu/drm/i915/intel_cdclk.c | 5 - > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c > > b/drivers/gpu/drm/i915/intel_cdclk.c > > index 11a0f3e122c3..fe7a9e3a4f29 100644 > > --- a/drivers/gpu/drm/i915/intel_cdclk.c > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > > @@ -494,6 +494,8 @@ static void vlv_set_cdclk(struct drm_i915_private > > *dev_priv, > > > > mutex_unlock(&dev_priv->sb_lock); > > > > + vlv_program_pfi_credits(dev_priv); > > + > > intel_update_cdclk(dev_priv); > > } > > > > @@ -533,6 +535,8 @@ static void chv_set_cdclk(struct drm_i915_private > > *dev_priv, > > } > > mutex_unlock(&dev_priv->rps.hw_lock); > > > > + vlv_program_pfi_credits(dev_priv); > > + > > intel_update_cdclk(dev_priv); > > Reviewed-by: Ander Conselvan de Oliveira > > Can the vlv/chv register write in intel_update_cdclk() be moved to the > set_cdclk() funcs too? We'll still need to initialize it at driver load time. I supoose I could also add it to the power well enable hook. IIRC this register probably had some magic retention logic while the power well is off so that we don't actually lose the state (the register is simply inaccessible while the power well is down). So for normal power well activity we wouldn't really need to rewrite the register every time, but I guess there'd not be much harm in doing it anyway. But I'll have to double check this stuff since I'm not 100% sure. > > > > } > > > > @@ -1433,7 +1437,6 @@ static void vlv_modeset_commit_cdclk(struct > > drm_atomic_state *old_state) > > else > > vlv_set_cdclk(dev_priv, &dev_priv->cdclk.actual); > > > > - vlv_program_pfi_credits(dev_priv); > > > > intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_A); > > } > > -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [GLK MIPI DSI V2 3/9] drm/i915/glk: Add MIPIIO Enable/disable sequence
On Thu, 15 Dec 2016, Madhav Chauhan wrote:
> From: Deepak M
>
> v2: Addressed Jani's Review comments(renamed bit field macros)
>
> Signed-off-by: Deepak M
> Signed-off-by: Madhav Chauhan
> ---
> drivers/gpu/drm/i915/intel_dsi.c | 134
> +++
> 1 file changed, 134 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> b/drivers/gpu/drm/i915/intel_dsi.c
> index b78c686..c0697e9 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -357,6 +357,134 @@ static bool intel_dsi_compute_config(struct
> intel_encoder *encoder,
> return true;
> }
>
> +static void intel_dsi_disable_mipiio(struct intel_encoder *encoder)
> +{
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> + enum port port;
> + u32 tmp;
> +
> + /* Put the IO into reset */
> + tmp = I915_READ(MIPI_CTRL(PORT_A));
> + tmp &= ~GLK_MIPIIO_RESET_RELEASED;
> + I915_WRITE(MIPI_CTRL(PORT_A), tmp);
> +
> + /* Wait for MIPI PHY status bit to unset */
> + for_each_dsi_port(port, intel_dsi->ports) {
> + if (intel_wait_for_register(dev_priv,
> + MIPI_CTRL(port),
> + GLK_PHY_STATUS_PORT_READY, 0, 20))
> + DRM_ERROR("PHY is not turning OFF\n");
> + }
> +
> + /* Clear MIPI mode */
> + for_each_dsi_port(port, intel_dsi->ports) {
> + tmp = I915_READ(MIPI_CTRL(port));
> + tmp &= ~GLK_MIPIIO_ENABLE;
> + I915_WRITE(MIPI_CTRL(port), tmp);
> + }
> +}
> +
> +static void intel_dsi_enable_mipiio(struct intel_encoder *encoder)
> +{
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> + enum port port;
> + u32 tmp, val;
> +
> + /* Put the IO into reset */
> + tmp = I915_READ(MIPI_CTRL(PORT_A));
> + tmp &= ~GLK_MIPIIO_RESET_RELEASED;
> + I915_WRITE(MIPI_CTRL(PORT_A), tmp);
> +
> + /* Program LP Wake */
> + for_each_dsi_port(port, intel_dsi->ports) {
> + tmp = I915_READ(MIPI_CTRL(port));
> + tmp &= ~GLK_LP_WAKE;
> + I915_WRITE(MIPI_CTRL(port), tmp);
> + }
> +
> + /* Set the MIPI mode */
> + for_each_dsi_port(port, intel_dsi->ports) {
> + tmp = I915_READ(MIPI_CTRL(port));
> + I915_WRITE(MIPI_CTRL(port), tmp | GLK_MIPIIO_ENABLE);
> + }
> +
> + /* Wait for Pwr ACK */
> + for_each_dsi_port(port, intel_dsi->ports) {
> + if (intel_wait_for_register(dev_priv,
> + MIPI_CTRL(port), GLK_MIPIIO_PORT_POWERED,
> + GLK_MIPIIO_PORT_POWERED, 20))
> + DRM_ERROR("Power ACK not received\n");
> + }
> +
> + /* Wait for MIPI PHY status bit to set */
> + for_each_dsi_port(port, intel_dsi->ports) {
> + if (intel_wait_for_register(dev_priv,
> + MIPI_CTRL(port), GLK_MIPIIO_PORT_POWERED,
> + GLK_MIPIIO_PORT_POWERED, 20))
> + DRM_ERROR("PHY is not ON\n");
> + }
> +
> + /* Get IO out of reset */
> + tmp = I915_READ(MIPI_CTRL(PORT_A));
> + I915_WRITE(MIPI_CTRL(PORT_A), tmp | GLK_MIPIIO_RESET_RELEASED);
> +
> + /* Get IO out of Low power state*/
> + for_each_dsi_port(port, intel_dsi->ports) {
> + if (!(I915_READ(MIPI_DEVICE_READY(port)) & DEVICE_READY)) {
> + val = I915_READ(MIPI_DEVICE_READY(port));
> + val &= ~ULPS_STATE_MASK;
> + val |= DEVICE_READY;
> + I915_WRITE(MIPI_DEVICE_READY(port), val);
> + usleep_range(10, 15);
> + }
> +
> + /* Enter ULPS */
> + val = I915_READ(MIPI_DEVICE_READY(port));
> + val &= ~ULPS_STATE_MASK;
> + val |= (ULPS_STATE_ENTER | DEVICE_READY);
> + I915_WRITE(MIPI_DEVICE_READY(port), val);
> +
> + /* Wait for ULPS Not active */
> + if (intel_wait_for_register(dev_priv,
> + MIPI_CTRL(port), GLK_ULPS_NOT_ACTIVE,
> + GLK_ULPS_NOT_ACTIVE, 20))
> +
> + /* Exit ULPS */
> + val = I915_READ(MIPI_DEVICE_READY(port));
> + val &= ~ULPS_STATE_MASK;
> + val |= (ULPS_STATE_EXIT | DEVICE_READY);
> + I915_WRITE(MIPI_DEVICE_READY(port), val);
> +
> + /* Enter Normal Mode */
> + val = I915_READ(MIPI_DEVICE_READY(port));
> + val &= ~ULPS_STATE_MASK;
> + val |= (ULPS_STATE_NORMAL_OPERATION | DEVICE_READY);
> + I915_WRITE(MIPI_DEVICE_READY(port), val);
> +
> + tmp = I915_READ(MIPI_CTRL(port));
> + tmp &= ~GLK_LP_WAKE;
> +
Re: [Intel-gfx] [PATCH 1/8] drm/i915/guc: Make the GuC fw loading helper functions general
On Thu, Dec 22, 2016 at 03:12:17PM -0800, Anusha Srivatsa wrote: > From: Peter Antoine > > Rename some of the GuC fw loading code to make them more general. We > will utilise them for HuC loading as well. > s/intel_guc_fw/intel_uc_fw/g > s/GUC_FIRMWARE/UC_FIRMWARE/g > > Struct intel_guc_fw is renamed to intel_uc_fw. Prefix of tts members, > such as 'guc' or 'guc_fw' either is renamed to 'uc' or removed for > same purpose. > > v2: rebased on top of nightly. > reapplied the search/replace as upstream code as changed. > v3: rebased again on drm-nightly. > v4: removed G from messages in shared fw fetch function. > v5: rebased. > v7: rebased. > v8: rebased. > v9: rebased. > v10: rebased. > v11: rebased. > v12: rebased on top of drm-tip > v13: rebased.Updated dev to dev_priv in intel_guc_setup(), guc_fw_getch() > and intel_guc_init(). > v14: rebased. Remove uint32_t fw_type to patch 2. Add INTEL_ prefix for > fields in enum intel_uc_fw_status. Remove uc_dev field since its never > used.Rename uc_fw to just fw and guc_fw to fw to avoid redundency. > v15: rebased. Remove sections of code that were commented and no longer > required. > > Signed-off-by: Anusha Srivatsa > Signed-off-by: Alex Dai > Signed-off-by: Peter Antoine Reviewed-by: Arkadiusz Hiler -- Cheers, Arek ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct for GuC and HuC
On Thu, Dec 22, 2016 at 03:12:18PM -0800, Anusha Srivatsa wrote:
> From: Peter Antoine
>
> HuC firmware css header has almost exactly same definition as GuC
> firmware except for the sw_version. Also, add a new member fw_type
> into intel_uc_fw to indicate what kind of fw it is. So, the loader
> will pull right sw_version from header.
>
> v2: rebased on-top of drm-intel-nightly
> v3: rebased on-top of drm-intel-nightly (again).
> v4: rebased + spaces.
> v7: rebased.
> v8: rebased.
> v9: rebased. Rename device_id to guc_branch_client_version,
> make guc_sw_version a union. . Put UC_FW_TYPE_GUC
> and UC_FW_TYPE_HUC into an enum.
> v10: rebased.
> v11: rebased.
> v12: rebased on top of drm-tip.
> v13: rebased.Update dev to dev_priv in intel_uc_fw_fetch
> v14: rebased. Add INTEL_ prefix to an enum. Add fw_type declaration
> from patch 1.Combine two different unions for huc and guc version,
> reserved etc into one union with two structs.
> v15: rebased.
>
> Tested-by: Xiang Haihao
> Signed-off-by: Anusha Srivatsa
> Signed-off-by: Alex Dai
> Signed-off-by: Peter Antoine
> ---
> drivers/gpu/drm/i915/intel_guc_fwif.h | 23 ++
> drivers/gpu/drm/i915/intel_guc_loader.c | 41
> ++---
> drivers/gpu/drm/i915/intel_uc.h | 6 +
> 3 files changed, 53 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h
> b/drivers/gpu/drm/i915/intel_guc_fwif.h
> index 3202b32..ed1ab40 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -145,7 +145,7 @@
> * The GuC firmware layout looks like this:
> *
> * +---+
> - * |guc_css_header |
> + * | uc_css_header |
> * | |
> * | contains major/minor version |
> * +---+
> @@ -172,9 +172,16 @@
> * 3. Length info of each component can be found in header, in dwords.
> * 4. Modulus and exponent key are not required by driver. They may not
> appear
> *in fw. So driver will load a truncated firmware in this case.
> + *
> + * HuC firmware layout is same as GuC firmware.
> + *
> + * HuC firmware css header is different. However, the only difference is
> where
> + * the version information is saved. The uc_css_header is unified to support
> + * both. Driver should get HuC version from uc_css_header.huc_sw_version,
> while
> + * uc_css_header.guc_sw_version for GuC.
> */
>
> -struct guc_css_header {
> +struct uc_css_header {
> uint32_t module_type;
> /* header_size includes all non-uCode bits, including css_header, rsa
>* key, modulus key and exponent data. */
> @@ -205,8 +212,16 @@ struct guc_css_header {
>
> char username[8];
> char buildnumber[12];
> - uint32_t device_id;
> - uint32_t guc_sw_version;
> + union {
> + struct {
> + uint32_t branch_client_version;
> + uint32_t sw_version;
> + } guc;
> + struct {
> + uint32_t sw_version;
> + uint32_t reserved;
> + } huc;
> + };
> uint32_t prod_preprod_fw;
> uint32_t reserved[12];
> uint32_t header_info;
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
> b/drivers/gpu/drm/i915/intel_guc_loader.c
> index ffe53dd7..06e3e5c 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -593,7 +593,7 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
> struct pci_dev *pdev = dev_priv->drm.pdev;
> struct drm_i915_gem_object *obj;
> const struct firmware *fw = NULL;
> - struct guc_css_header *css;
> + struct uc_css_header *css;
> size_t size;
> int err;
>
> @@ -610,19 +610,19 @@ void intel_uc_fw_fetch(struct drm_i915_private
> *dev_priv,
> uc_fw->uc_fw_path, fw);
>
> /* Check the size of the blob before examining buffer contents */
> - if (fw->size < sizeof(struct guc_css_header)) {
> + if (fw->size < sizeof(struct uc_css_header)) {
> DRM_NOTE("Firmware header is missing\n");
> goto fail;
> }
>
> - css = (struct guc_css_header *)fw->data;
> + css = (struct uc_css_header *)fw->data;
>
> /* Firmware bits always start from header */
> uc_fw->header_offset = 0;
> uc_fw->header_size = (css->header_size_dw - css->modulus_size_dw -
> css->key_size_dw - css->exponent_size_dw) * sizeof(u32);
>
> - if (uc_fw->header_size != sizeof(struct guc_css_header)) {
> + if (uc_fw->header_size != sizeof(struct uc_css_header)) {
> DRM_NOTE("CSS header definition mismatch\n");
> goto fail;
> }
> @@ -646,21 +646,36 @@ void intel_uc_fw_fetch(struct drm_i915_private
> *dev_priv,
> goto fail;
> }
>
> - /* Header and u
Re: [Intel-gfx] [PATCH 1/2] drm : adds Y-coordinate and Colorimetry Format
In DP V1.3 spec , Table 2-149 , page number-374 , for Register 0x2210 , bit 7:4 is reserved. -Original Message- From: Jani Nikula [mailto:jani.nik...@linux.intel.com] Sent: Friday, December 23, 2016 6:57 PM To: Nagaraju, Vathsala ; dri-de...@lists.freedesktop.org; intel-gfx@lists.freedesktop.org Cc: Vivi, Rodrigo Subject: Re: [Intel-gfx] [PATCH 1/2] drm : adds Y-coordinate and Colorimetry Format On Thu, 22 Dec 2016, vathsala nagaraju wrote: > PSR2 vsc revision number hb2( as per table 6-11)is updated to > 4 or 5 based on Y cordinate and Colorimetry Format as below 04h = 3D > stereo + PSR/PSR2 + Y-coordinate. > 05h = -3D stereo- + PSR/PSR2 + Y-coordinate + Pixel > Encoding/Colorimetry Format indication. A DP Source device is allowed > to indicate the pixel encoding/colorimetry format to the DP Sink > device with VSC SDP only when the DP Sink device supports it ( > i.e.,VSC_SDP_EXTENSION_FOR_COLORIMETRY_SUPPORTED bit in the > DPRX_FEATURE_ENUMERATION_LIST register (DPCD Address 02210h, bit 3; is > set to 1). > > v2: (Jani) > - Change DP_PSR_Y_COORDINATE to DP_PSR2_SU_Y_COORDINATE_REQUIRED. > - Add DP_PSR2_SU_GRANULARITY_REQUIRED. > - Change DPRX_FEATURE_ENUMERATION_LIST to DP_DPRX. > - Add GTC_CAP and AV_SYNC_CAP, other bits in DPRX_FEATURE_ENUMERATION_LIST. > > Cc: Rodrigo Vivi > Cc: Jim Bride > Signed-off-by: Vathsala Nagaraju > --- > include/drm/drm_dp_helper.h | 8 +++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h > index 55bbeb0..ee2a649d 100644 > --- a/include/drm/drm_dp_helper.h > +++ b/include/drm/drm_dp_helper.h > @@ -194,7 +194,8 @@ > # define DP_PSR_SETUP_TIME_0(6 << 1) > # define DP_PSR_SETUP_TIME_MASK (7 << 1) > # define DP_PSR_SETUP_TIME_SHIFT1 > - > +# define DP_PSR2_SU_Y_COORDINATE_REQUIRED (1 << 4) /* eDP 1.4a */ > +# define DP_PSR2_SU_GRANULARITY_REQUIRED(1 << 5) /* eDP 1.4b */ > /* > * 0x80-0x8f describe downstream port capabilities, but there are two layouts > * based on whether DP_DETAILED_CAP_INFO_AVAILABLE was set. If it > was not, @@ -568,6 +569,11 @@ > #define DP_RECEIVER_ALPM_STATUS 0x200b /* eDP 1.4 */ > # define DP_ALPM_LOCK_TIMEOUT_ERROR (1 << 0) > > +#define DP_DPRX_FEATURE_ENUMERATION_LIST0x2210 > +# define DP_GTC_CAP (1 << 0) > +# define DP_AV_SYNC_CAP (1 << 2) > +# define DP_VSC_SDP_EXT_FOR_COLORIMETRY_SUPPORTED(1 << 3) The spec continues with bits 4 to 7 in the next page... > + > /* DP 1.2 Sideband message defines */ > /* peer device type - DP 1.2a Table 2-92 */ > #define DP_PEER_DEVICE_NONE 0x0 -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams
On Thu, Dec 22, 2016 at 03:12:24PM -0800, Anusha Srivatsa wrote: > From: Peter Antoine > > This patch will allow for getparams to return the status of the HuC. > As the HuC has to be validated by the GuC this patch uses the validated > status to show when the HuC is loaded and ready for use. You cannot use > the loaded status as with the GuC as the HuC is verified after it is > loaded and is not usable until it is verified. > > v2: removed the forewakes as the registers are already force-woken. > (T.Ursulin) > v4: rebased. > v5: rebased on top of drm-tip. > v6: rebased. Removed any reference to intel_huc.h > v7: rebased. Rename I915_PARAM_HAS_HUC to I915_PARAM_HUC_STATUS. > Remove intel_is_huc_valid() since it is used only in one place. > Put the case of I915_PARAM_HAS_HUC() in the right place. > v8: rebased. Add a comment to specify that I915_READ(reg) > does not read garbage value. The register HUC_STATUS2 is force > woken and no rpm is needed. > > Signed-off-by: Peter Antoine Where is your s-o-b? other than that: Reviewed-by: Arkadiusz Hiler -- Cheers, Arek ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/8] drm/i915/HuC: Add KBL huC loading Support
On Thu, Dec 22, 2016 at 03:12:21PM -0800, Anusha Srivatsa wrote: > This patch adds the support to load HuC on KBL > Version 2.0 > > v2: rebased. > v3: rebased on top of drm-tip > v4: rebased. > v5: rebased. Rename KBL_FW_ to KBL_HUC_FW_ > v6: rebased. Remove old checks. > > Cc: Tvrtko Ursulin > Signed-off-by: Anusha Srivatsa Reviewed-by: Arkadiusz Hiler -- Cheers, Arek ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/8] drm/i915/huc: Add BXT HuC Loading Support
On Thu, Dec 22, 2016 at 03:12:20PM -0800, Anusha Srivatsa wrote: > This patch adds the HuC Loading for the BXT by using > the updated file construction. > > Version 1.7 of the HuC firmware. > > v2: rebased. > v3: rebased on top of drm-tip > v4: rebased. > v5: rebased. Rename BXT_FW_MAJOR to BXT_HUC_FW_ > v6: rebased. > > Cc: Tvrtko Ursulin > Signed-off-by: Anusha Srivatsa Reviewed-by: Arkadiusz Hiler -- Cheers, Arek ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 11/11] drm/i915: Kill the 830 MI_OVERLAY_OFF workaround
On Thu, Dec 22, 2016 at 09:40:41PM +, Chris Wilson wrote: > On Thu, Dec 22, 2016 at 09:52:22PM +0200, ville.syrj...@linux.intel.com wrote: > > From: Ville Syrjälä > > > > Now that we're disabling L2 clock gating MI_OVERLAY_OFF actually works > > on 830, so let's use it. > > > > v2: Nuke the unused dev_priv variable > > > > Signed-off-by: Ville Syrjälä > > Does what it says on the tin, and it does look consistent with the > earlier patch for L2 clock gating, so > > Reviewed-by: Chris Wilson Thanks. Pushed as well. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 01/10] drm/i915: Break after walking all GGTT vma in bump_inactive_ggtt
Since commit db6c2b4151f2 ("drm/i915: Store the vma in an rbtree under
the object") the vma are once again sorted into GGTT first, then ppGTT
so that the typical case of walking the GGTT vma can stop as soon as we
find a non-ppGTT. Apply that optimisation.
Signed-off-by: Chris Wilson
Cc: Tvrtko Ursulin
Cc: Joonas Lahtinen
Reviewed-by: Joonas Lahtinen
---
drivers/gpu/drm/i915/i915_gem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 613a92414a00..5a08e8c187e8 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1515,7 +1515,7 @@ static void i915_gem_object_bump_inactive_ggtt(struct
drm_i915_gem_object *obj)
list_for_each_entry(vma, &obj->vma_list, obj_link) {
if (!i915_vma_is_ggtt(vma))
- continue;
+ break;
if (i915_vma_is_active(vma))
continue;
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 03/10] drm/i915: Don't clflush before release phys object
When we teardown the backing storage for the phys object, we copy from
the coherent contiguous block back to the shmemfs object, clflushing as
we go. Trying to clflush the invalid sg beforehand just oops and would
be redundant (due to it already being coherent, and clflushed
afterwards).
Reported-by: Ville Syrjälä
Signed-off-by: Chris Wilson
Cc: Joonas Lahtinen
Cc:
Reviewed-by: Joonas Lahtinen
---
drivers/gpu/drm/i915/i915_gem.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index bdb37bd3813b..09185d431b52 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -246,14 +246,16 @@ i915_gem_object_get_pages_phys(struct drm_i915_gem_object
*obj)
static void
__i915_gem_object_release_shmem(struct drm_i915_gem_object *obj,
- struct sg_table *pages)
+ struct sg_table *pages,
+ bool needs_clflush)
{
GEM_BUG_ON(obj->mm.madv == __I915_MADV_PURGED);
if (obj->mm.madv == I915_MADV_DONTNEED)
obj->mm.dirty = false;
- if ((obj->base.read_domains & I915_GEM_DOMAIN_CPU) == 0 &&
+ if (needs_clflush &&
+ (obj->base.read_domains & I915_GEM_DOMAIN_CPU) == 0 &&
!cpu_cache_is_coherent(obj->base.dev, obj->cache_level))
drm_clflush_sg(pages);
@@ -265,7 +267,7 @@ static void
i915_gem_object_put_pages_phys(struct drm_i915_gem_object *obj,
struct sg_table *pages)
{
- __i915_gem_object_release_shmem(obj, pages);
+ __i915_gem_object_release_shmem(obj, pages, false);
if (obj->mm.dirty) {
struct address_space *mapping = obj->base.filp->f_mapping;
@@ -2232,7 +2234,7 @@ i915_gem_object_put_pages_gtt(struct drm_i915_gem_object
*obj,
struct sgt_iter sgt_iter;
struct page *page;
- __i915_gem_object_release_shmem(obj, pages);
+ __i915_gem_object_release_shmem(obj, pages, true);
i915_gem_gtt_finish_pages(obj, pages);
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 06/10] drm/i915: Convert i915_ggtt_view to use an anonymous union
Save a lot of characters by making the union anonymous, with the
side-effect of ignoring unset bits when comparing views.
v2: Roll up the memcmps back into one.
Signed-off-by: Chris Wilson
Cc: Daniel Vetter
---
drivers/gpu/drm/i915/i915_gem.c | 8
drivers/gpu/drm/i915/i915_gem_gtt.c | 9 -
drivers/gpu/drm/i915/i915_gem_gtt.h | 2 +-
drivers/gpu/drm/i915/i915_vma.c | 9 -
drivers/gpu/drm/i915/i915_vma.h | 18 --
drivers/gpu/drm/i915/intel_display.c | 2 +-
6 files changed, 26 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1fa86e3f6440..1281f0fa45f6 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1866,10 +1866,10 @@ int i915_gem_fault(struct vm_area_struct *area, struct
vm_fault *vmf)
memset(&view, 0, sizeof(view));
view.type = I915_GGTT_VIEW_PARTIAL;
- view.params.partial.offset = rounddown(page_offset, chunk_size);
- view.params.partial.size =
+ view.partial.offset = rounddown(page_offset, chunk_size);
+ view.partial.size =
min_t(unsigned int, chunk_size,
- vma_pages(area) - view.params.partial.offset);
+ vma_pages(area) - view.partial.offset);
/* If the partial covers the entire object, just create a
* normal VMA.
@@ -1904,7 +1904,7 @@ int i915_gem_fault(struct vm_area_struct *area, struct
vm_fault *vmf)
/* Finally, remap it using the new GTT offset */
ret = remap_io_mapping(area,
- area->vm_start +
(vma->ggtt_view.params.partial.offset << PAGE_SHIFT),
+ area->vm_start + (vma->ggtt_view.partial.offset
<< PAGE_SHIFT),
(ggtt->mappable_base + vma->node.start) >>
PAGE_SHIFT,
min_t(u64, vma->size, area->vm_end -
area->vm_start),
&ggtt->mappable);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 6af9311f72f5..7903a7480715 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3466,7 +3466,7 @@ intel_partial_pages(const struct i915_ggtt_view *view,
{
struct sg_table *st;
struct scatterlist *sg, *iter;
- unsigned int count = view->params.partial.size;
+ unsigned int count = view->partial.size;
unsigned int offset;
int ret = -ENOMEM;
@@ -3478,9 +3478,7 @@ intel_partial_pages(const struct i915_ggtt_view *view,
if (ret)
goto err_sg_alloc;
- iter = i915_gem_object_get_sg(obj,
- view->params.partial.offset,
- &offset);
+ iter = i915_gem_object_get_sg(obj, view->partial.offset, &offset);
GEM_BUG_ON(!iter);
sg = st->sgl;
@@ -3532,7 +3530,8 @@ i915_get_ggtt_vma_pages(struct i915_vma *vma)
vma->pages = vma->obj->mm.pages;
else if (vma->ggtt_view.type == I915_GGTT_VIEW_ROTATED)
vma->pages =
-
intel_rotate_fb_obj_pages(&vma->ggtt_view.params.rotated, vma->obj);
+ intel_rotate_fb_obj_pages(&vma->ggtt_view.rotated,
+ vma->obj);
else if (vma->ggtt_view.type == I915_GGTT_VIEW_PARTIAL)
vma->pages = intel_partial_pages(&vma->ggtt_view, vma->obj);
else
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 0055b8567a43..f3d71b6cc702 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -164,7 +164,7 @@ struct i915_ggtt_view {
unsigned int size;
} partial;
struct intel_rotation_info rotated;
- } params;
+ };
};
extern const struct i915_ggtt_view i915_ggtt_view_normal;
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 868d061e1a11..b7d7eb1540a9 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -96,15 +96,14 @@ __i915_vma_create(struct drm_i915_gem_object *obj,
vma->ggtt_view = *view;
if (view->type == I915_GGTT_VIEW_PARTIAL) {
GEM_BUG_ON(range_overflows_t(u64,
-
view->params.partial.offset,
-view->params.partial.size,
+view->partial.offset,
+view->partial.size,
obj->base.size >>
PAGE_SHIFT));
- vma->size = view->params
[Intel-gfx] [PATCH 07/10] drm/i915: Eliminate superfluous i915_ggtt_view_rotated
It is only being used to clear a struct and set the type, after which it
is overwritten. Since we no longer check the unset bits of the union,
skipping the clear is permissible.
Signed-off-by: Chris Wilson
Reviewed-by: Joonas Lahtinen
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 3 ---
drivers/gpu/drm/i915/i915_gem_gtt.h | 1 -
drivers/gpu/drm/i915/intel_display.c | 5 ++---
3 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 7903a7480715..f60665459802 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -102,9 +102,6 @@ i915_get_ggtt_vma_pages(struct i915_vma *vma);
const struct i915_ggtt_view i915_ggtt_view_normal = {
.type = I915_GGTT_VIEW_NORMAL,
};
-const struct i915_ggtt_view i915_ggtt_view_rotated = {
- .type = I915_GGTT_VIEW_ROTATED,
-};
int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
int enable_ppgtt)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index f3d71b6cc702..513fe4db7899 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -168,7 +168,6 @@ struct i915_ggtt_view {
};
extern const struct i915_ggtt_view i915_ggtt_view_normal;
-extern const struct i915_ggtt_view i915_ggtt_view_rotated;
enum i915_cache_level;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 5869efea22fa..26c0947fdc6e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2138,11 +2138,10 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view,
const struct drm_framebuffer *fb,
unsigned int rotation)
{
+ view->type = I915_GGTT_VIEW_NORMAL;
if (drm_rotation_90_or_270(rotation)) {
- *view = i915_ggtt_view_rotated;
+ view->type = I915_GGTT_VIEW_ROTATED;
view->rotated = to_intel_framebuffer(fb)->rot_info;
- } else {
- *view = i915_ggtt_view_normal;
}
}
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 05/10] drm/i915: Assert that the partial VMA fits within the object
When creating a partial VMA assert that it first fits with the parent
object, and that if it covers the whole of the parent a normal view was
created instead.
Signed-off-by: Chris Wilson
Reviewed-by: Joonas Lahtinen
---
drivers/gpu/drm/i915/i915_vma.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index fd75d5704287..868d061e1a11 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -95,8 +95,13 @@ __i915_vma_create(struct drm_i915_gem_object *obj,
if (view) {
vma->ggtt_view = *view;
if (view->type == I915_GGTT_VIEW_PARTIAL) {
+ GEM_BUG_ON(range_overflows_t(u64,
+
view->params.partial.offset,
+view->params.partial.size,
+obj->base.size >>
PAGE_SHIFT));
vma->size = view->params.partial.size;
vma->size <<= PAGE_SHIFT;
+ GEM_BUG_ON(vma->size >= obj->base.size);
} else if (view->type == I915_GGTT_VIEW_ROTATED) {
vma->size =
intel_rotation_info_size(&view->params.rotated);
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 10/10] drm/i915: Prevent timeline updates whilst performing reset
As the fence may be signaled concurrently from an interrupt on another
device, it is possible for the list of requests on the timeline to be
modified as we walk it. Take both (the context's timeline and the global
timeline) locks to prevent such modifications.
Fixes: 80b204bce8f2 ("drm/i915: Enable multiple timelines")
Signed-off-by: Chris Wilson
Cc: Joonas Lahtinen
Cc: Mika Kuoppala
Cc:
Reviewed-by: Mika Kuoppala
---
drivers/gpu/drm/i915/i915_gem.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 94769e3556aa..dba4daaefb94 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2751,6 +2751,7 @@ static void i915_gem_reset_engine(struct intel_engine_cs
*engine)
struct drm_i915_gem_request *request;
struct i915_gem_context *incomplete_ctx;
struct intel_timeline *timeline;
+ unsigned long flags;
bool ring_hung;
if (engine->irq_seqno_barrier)
@@ -2794,13 +2795,20 @@ static void i915_gem_reset_engine(struct
intel_engine_cs *engine)
if (i915_gem_context_is_default(incomplete_ctx))
return;
+ timeline = i915_gem_context_lookup_timeline(incomplete_ctx, engine);
+
+ spin_lock_irqsave(&engine->timeline->lock, flags);
+ spin_lock(&timeline->lock);
+
list_for_each_entry_continue(request, &engine->timeline->requests, link)
if (request->ctx == incomplete_ctx)
reset_request(request);
- timeline = i915_gem_context_lookup_timeline(incomplete_ctx, engine);
list_for_each_entry(request, &timeline->requests, link)
reset_request(request);
+
+ spin_unlock(&timeline->lock);
+ spin_unlock_irqrestore(&engine->timeline->lock, flags);
}
void i915_gem_reset(struct drm_i915_private *dev_priv)
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 09/10] drm/i915: Extact compute_partial_view()
In order to reuse the partial view for selftesting, extract the common
function for computing the view.
Signed-off-by: Chris Wilson
Reviewed-by: Joonas Lahtinen
---
drivers/gpu/drm/i915/i915_gem.c | 48 +
1 file changed, 29 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2f1f01d2d768..94769e3556aa 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1780,6 +1780,32 @@ int i915_gem_mmap_gtt_version(void)
return 1;
}
+static inline struct i915_ggtt_view
+compute_partial_view(struct drm_i915_gem_object *obj,
+struct vm_area_struct *area,
+pgoff_t page_offset,
+unsigned int chunk_size)
+{
+ struct i915_ggtt_view view;
+
+ if (i915_gem_object_is_tiled(obj))
+ chunk_size = roundup(chunk_size, tile_row_pages(obj));
+
+ view.type = I915_GGTT_VIEW_PARTIAL;
+ view.partial.offset = rounddown(page_offset, chunk_size);
+ view.partial.size =
+ min_t(unsigned int, chunk_size,
+ vma_pages(area) - view.partial.offset);
+
+ /* If the partial covers the entire object, just create a
+* normal VMA.
+*/
+ if (chunk_size >= obj->base.size >> PAGE_SHIFT)
+ view.type = I915_GGTT_VIEW_NORMAL;
+
+ return view;
+}
+
/**
* i915_gem_fault - fault a page into the GTT
* @area: CPU VMA in question
@@ -1856,26 +1882,10 @@ int i915_gem_fault(struct vm_area_struct *area, struct
vm_fault *vmf)
/* Now pin it into the GTT as needed */
vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, flags);
if (IS_ERR(vma)) {
- struct i915_ggtt_view view;
- unsigned int chunk_size;
-
/* Use a partial view if it is bigger than available space */
- chunk_size = MIN_CHUNK_PAGES;
- if (i915_gem_object_is_tiled(obj))
- chunk_size = roundup(chunk_size, tile_row_pages(obj));
-
- memset(&view, 0, sizeof(view));
- view.type = I915_GGTT_VIEW_PARTIAL;
- view.partial.offset = rounddown(page_offset, chunk_size);
- view.partial.size =
- min_t(unsigned int, chunk_size,
- vma_pages(area) - view.partial.offset);
-
- /* If the partial covers the entire object, just create a
-* normal VMA.
-*/
- if (chunk_size >= obj->base.size >> PAGE_SHIFT)
- view.type = I915_GGTT_VIEW_NORMAL;
+ struct i915_ggtt_view view =
+ compute_partial_view(obj, area,
+page_offset, MIN_CHUNK_PAGES);
/* Userspace is now writing through an untracked VMA, abandon
* all hope that the hardware is able to track future writes.
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 02/10] drm/i915: Repeat flush of idle work during suspend
The idle work handler is self-arming - if it detects that it needs to
run again it will queue itself from its work handler. Take greater care
when trying to drain the idle work, and double check that it is flushed.
The free worker has a similar issue where it is armed by an RCU task
which may be running concurrently with us.
This should hopefully help with the sporadic WARN_ON(dev_priv->gt.awake)
from i915_gem_suspend.
v2: Reuse drain_freed_objects.
v3: Don't try to flush the freed objects from the shrinker, as it may be
underneath the struct_mutex already.
v4: do while and comment upon the excess rcu_barrier in drain_freed_objects
Signed-off-by: Chris Wilson
Reviewed-by: Joonas Lahtinen
---
drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
drivers/gpu/drm/i915/i915_drv.c | 3 +--
drivers/gpu/drm/i915/i915_drv.h | 13 +
drivers/gpu/drm/i915/i915_gem.c | 10 --
4 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index a5552a1ca01f..655b6719c3da 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4124,7 +4124,7 @@ i915_drop_caches_set(void *data, u64 val)
if (val & DROP_FREED) {
synchronize_rcu();
- flush_work(&dev_priv->mm.free_work);
+ i915_gem_drain_freed_objects(dev_priv);
}
return ret;
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 6428588518aa..2c020eafada6 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -545,8 +545,7 @@ static void i915_gem_fini(struct drm_i915_private *dev_priv)
i915_gem_context_fini(dev_priv);
mutex_unlock(&dev_priv->drm.struct_mutex);
- rcu_barrier();
- flush_work(&dev_priv->mm.free_work);
+ i915_gem_drain_freed_objects(dev_priv);
WARN_ON(!list_empty(&dev_priv->context_list));
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1a914095efdd..3794b0933531 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3208,6 +3208,19 @@ i915_gem_object_create_from_data(struct drm_i915_private
*dev_priv,
void i915_gem_close_object(struct drm_gem_object *gem, struct drm_file *file);
void i915_gem_free_object(struct drm_gem_object *obj);
+static inline void i915_gem_drain_freed_objects(struct drm_i915_private *i915)
+{
+ /* A single pass should suffice to release all the freed objects (along
+* most call paths) , but be a little more paranoid in that freeing
+* the objects does take a little amount of time, during which the rcu
+* callbacks could have added new objects into the freed list, and
+* armed the work again.
+*/
+ do {
+ rcu_barrier();
+ } while (flush_work(&i915->mm.free_work));
+}
+
struct i915_vma * __must_check
i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
const struct i915_ggtt_view *view,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 5a08e8c187e8..bdb37bd3813b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4263,8 +4263,14 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work);
cancel_delayed_work_sync(&dev_priv->gt.retire_work);
- flush_delayed_work(&dev_priv->gt.idle_work);
- flush_work(&dev_priv->mm.free_work);
+
+ /* As the idle_work is rearming if it detects a race, play safe and
+* repeat the flush until it is definitely idle.
+*/
+ while (flush_delayed_work(&dev_priv->gt.idle_work))
+ ;
+
+ i915_gem_drain_freed_objects(dev_priv);
/* Assert that we sucessfully flushed all the work and
* reset the GPU back to its idle, low power state.
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 04/10] drm/i915: Silence allocation failure during sg_trim()
As trimming the sg table is merely an optimisation that gracefully fails
if we cannot allocate a new table, we do not need to report the failure
either.
Fixes: 0c40ce130e38 ("drm/i915: Trim the object sg table")
Signed-off-by: Chris Wilson
Cc: Tvrtko Ursulin
Reviewed-by: Joonas Lahtinen
---
drivers/gpu/drm/i915/i915_gem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 09185d431b52..1fa86e3f6440 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2325,7 +2325,7 @@ static void i915_sg_trim(struct sg_table *orig_st)
if (orig_st->nents == orig_st->orig_nents)
return;
- if (sg_alloc_table(&new_st, orig_st->nents, GFP_KERNEL))
+ if (sg_alloc_table(&new_st, orig_st->nents, GFP_KERNEL | __GFP_NOWARN))
return;
new_sg = new_st.sgl;
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 08/10] drm/i915: Eliminate superfluous i915_ggtt_view_normal
Since commit 058d88c4330f ("drm/i915: Track pinned VMA"), there is only
one user of i915_ggtt_view_normal rodate. Just treat NULL as no special
view in pin_to_display() like everywhere else.
Signed-off-by: Chris Wilson
Reviewed-by: Joonas Lahtinen
---
drivers/gpu/drm/i915/i915_gem.c | 2 +-
drivers/gpu/drm/i915/i915_gem_gtt.c | 4
drivers/gpu/drm/i915/i915_gem_gtt.h | 2 --
drivers/gpu/drm/i915/intel_overlay.c | 3 +--
4 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1281f0fa45f6..2f1f01d2d768 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3514,7 +3514,7 @@ i915_gem_object_pin_to_display_plane(struct
drm_i915_gem_object *obj,
* try to preserve the existing ABI).
*/
vma = ERR_PTR(-ENOSPC);
- if (view->type == I915_GGTT_VIEW_NORMAL)
+ if (!view || view->type == I915_GGTT_VIEW_NORMAL)
vma = i915_gem_object_ggtt_pin(obj, view, 0, alignment,
PIN_MAPPABLE | PIN_NONBLOCK);
if (IS_ERR(vma)) {
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index f60665459802..a67746775f71 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -99,10 +99,6 @@
static int
i915_get_ggtt_vma_pages(struct i915_vma *vma);
-const struct i915_ggtt_view i915_ggtt_view_normal = {
- .type = I915_GGTT_VIEW_NORMAL,
-};
-
int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
int enable_ppgtt)
{
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 513fe4db7899..d9860669082c 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -167,8 +167,6 @@ struct i915_ggtt_view {
};
};
-extern const struct i915_ggtt_view i915_ggtt_view_normal;
-
enum i915_cache_level;
struct i915_vma;
diff --git a/drivers/gpu/drm/i915/intel_overlay.c
b/drivers/gpu/drm/i915/intel_overlay.c
index 4473a611c664..0608fad7f593 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -811,8 +811,7 @@ static int intel_overlay_do_put_image(struct intel_overlay
*overlay,
if (ret != 0)
return ret;
- vma = i915_gem_object_pin_to_display_plane(new_bo, 0,
- &i915_ggtt_view_normal);
+ vma = i915_gem_object_pin_to_display_plane(new_bo, 0, NULL);
if (IS_ERR(vma))
return PTR_ERR(vma);
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: rewrite FBC's atomic CRTC-choosing code
Ville pointed out that intel_fbc_choose_crtc() is iterating over all
planes instead of just the primary planes. There are no real
consequences of this problem for HSW+, and for the other platforms it
just means that in some obscure multi-screen cases we'll keep FBC
disabled when we could have enabled it. Still, iterating over all
planes doesn't seem to be the best thing to do.
My initial idea was to just add a check for plane->type and be done,
but then I realized that in commits not involving the primary plane we
would reset crtc_state->enable_fbc back to false even when FBC is
enabled. That also wouldn't result in a bug due to the way the
enable_fbc variable is checked, but, still, our code can be better
than this.
So I went for the solution that involves tracking enable_fbc in the
primary plane state instead of the CRTC state. This way, if a commit
doesn't involve the primary plane for the CRTC we won't be resetting
enable_fbc back to false, so the variable will always reflect the
reality. And this change also makes more sense since FBC is actually
tied to the single plane and not the full pipe. As a bonus, we only
iterate over the CRTCs instead of iterating over all planes.
v2:
But Ville pointed that this only works properly if we have a single
commit with multiple CRTCs. If we have multiple parallel commits, each
with its own CRTC, we'll end with enable_fbc set to true in more than
one plane.
So the solution was to rename enable_fbc to can_enable_fbc. If we just
did the rename as the patch was, we'd end up with a single plane with
can_enable_fbc on commits involving multiple CRTCs: we'd choose the
best one, but we'd still end up with a variable that doesn't 100%
reflect reality.
So in the end I adopted Ville's suggestion of the fbc_score variable.
And then, instead of checking the score at intel_fbc_choose_crtc()
it should be possible to check for the score at intel_fbc_enable().
This allows us to simplify intel_fbc_choose_crtc() to the point where
it only needs to look at the given plane in order to assign its score
instead of looking at every primary plane on the same commit.
We still only set scores of 0 and 1 and we don't really do the
score-checking loop.
v3: Rebase.
v4:
Move intel_fbc_check_plane() call up so it will still zero the score
if the plane has no FB. (Ville).
Cc: Ville Syrjälä
Reported-by: Ville Syrjälä
Signed-off-by: Paulo Zanoni
---
drivers/gpu/drm/i915/intel_display.c | 3 +-
drivers/gpu/drm/i915/intel_drv.h | 8 ++--
drivers/gpu/drm/i915/intel_fbc.c | 85
3 files changed, 35 insertions(+), 61 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index d8effd4..56ae32c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14191,7 +14191,6 @@ static int intel_atomic_check(struct drm_device *dev,
if (ret)
return ret;
- intel_fbc_choose_crtc(dev_priv, state);
return calc_watermark_data(state);
}
@@ -14957,6 +14956,8 @@ intel_check_primary_plane(struct drm_plane *plane,
if (ret)
return ret;
+ intel_fbc_check_plane(state);
+
if (!state->base.fb)
return 0;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 025e4c8..7430082 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -406,6 +406,9 @@ struct intel_plane_state {
*/
int scaler_id;
+ /* 0: not suitable for FBC, 1+: suitable for FBC, more is better. */
+ unsigned int fbc_score;
+
struct drm_intel_sprite_colorkey ckey;
};
@@ -652,8 +655,6 @@ struct intel_crtc_state {
bool ips_enabled;
- bool enable_fbc;
-
bool double_wide;
int pbn;
@@ -1535,8 +1536,7 @@ static inline void intel_fbdev_restore_mode(struct
drm_device *dev)
#endif
/* intel_fbc.c */
-void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv,
- struct drm_atomic_state *state);
+void intel_fbc_check_plane(struct intel_plane_state *plane_state);
bool intel_fbc_is_active(struct drm_i915_private *dev_priv);
void intel_fbc_pre_update(struct intel_crtc *crtc,
struct intel_crtc_state *crtc_state,
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 26a81a9..ceda6f4 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -1040,68 +1040,32 @@ void intel_fbc_flush(struct drm_i915_private *dev_priv,
}
/**
- * intel_fbc_choose_crtc - select a CRTC to enable FBC on
- * @dev_priv: i915 device instance
- * @state: the atomic state structure
+ * intel_fbc_check_plane - check plane for FBC suitability
+ * @plane_state: the plane state
*
- * This function looks at the proposed state for CRTCs and planes, then chooses
- * which pipe is going to have FBC by setting i
Re: [Intel-gfx] [PATCH 2/2] drm/i915: rewrite FBC's atomic CRTC-choosing code
Em Sex, 2016-12-23 às 13:29 -0200, Paulo Zanoni escreveu:
> Ville pointed out that intel_fbc_choose_crtc() is iterating over all
> planes instead of just the primary planes. There are no real
> consequences of this problem for HSW+, and for the other platforms it
> just means that in some obscure multi-screen cases we'll keep FBC
> disabled when we could have enabled it. Still, iterating over all
> planes doesn't seem to be the best thing to do.
>
> My initial idea was to just add a check for plane->type and be done,
> but then I realized that in commits not involving the primary plane
> we
> would reset crtc_state->enable_fbc back to false even when FBC is
> enabled. That also wouldn't result in a bug due to the way the
> enable_fbc variable is checked, but, still, our code can be better
> than this.
>
> So I went for the solution that involves tracking enable_fbc in the
> primary plane state instead of the CRTC state. This way, if a commit
> doesn't involve the primary plane for the CRTC we won't be resetting
> enable_fbc back to false, so the variable will always reflect the
> reality. And this change also makes more sense since FBC is actually
> tied to the single plane and not the full pipe. As a bonus, we only
> iterate over the CRTCs instead of iterating over all planes.
>
> v2:
>
> But Ville pointed that this only works properly if we have a single
> commit with multiple CRTCs. If we have multiple parallel commits,
> each
> with its own CRTC, we'll end with enable_fbc set to true in more than
> one plane.
>
> So the solution was to rename enable_fbc to can_enable_fbc. If we
> just
> did the rename as the patch was, we'd end up with a single plane with
> can_enable_fbc on commits involving multiple CRTCs: we'd choose the
> best one, but we'd still end up with a variable that doesn't 100%
> reflect reality.
>
> So in the end I adopted Ville's suggestion of the fbc_score variable.
> And then, instead of checking the score at intel_fbc_choose_crtc()
> it should be possible to check for the score at intel_fbc_enable().
> This allows us to simplify intel_fbc_choose_crtc() to the point where
> it only needs to look at the given plane in order to assign its score
> instead of looking at every primary plane on the same commit.
>
> We still only set scores of 0 and 1 and we don't really do the
> score-checking loop.
>
> v3: Rebase.
>
> v4:
>
> Move intel_fbc_check_plane() call up so it will still zero the score
> if the plane has no FB. (Ville).
Oh, ok, so after moving the call up the case where the CRTC is NULL can
actually happen. Previously it was prevented because we always ran the
function when we had an FB.
I recall this patch. Shouldn't have sent it so soon.
>
> Cc: Ville Syrjälä
> Reported-by: Ville Syrjälä
> Signed-off-by: Paulo Zanoni
> ---
> drivers/gpu/drm/i915/intel_display.c | 3 +-
> drivers/gpu/drm/i915/intel_drv.h | 8 ++--
> drivers/gpu/drm/i915/intel_fbc.c | 85
>
> 3 files changed, 35 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index d8effd4..56ae32c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14191,7 +14191,6 @@ static int intel_atomic_check(struct
> drm_device *dev,
> if (ret)
> return ret;
>
> - intel_fbc_choose_crtc(dev_priv, state);
> return calc_watermark_data(state);
> }
>
> @@ -14957,6 +14956,8 @@ intel_check_primary_plane(struct drm_plane
> *plane,
> if (ret)
> return ret;
>
> + intel_fbc_check_plane(state);
> +
> if (!state->base.fb)
> return 0;
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 025e4c8..7430082 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -406,6 +406,9 @@ struct intel_plane_state {
> */
> int scaler_id;
>
> + /* 0: not suitable for FBC, 1+: suitable for FBC, more is
> better. */
> + unsigned int fbc_score;
> +
> struct drm_intel_sprite_colorkey ckey;
> };
>
> @@ -652,8 +655,6 @@ struct intel_crtc_state {
>
> bool ips_enabled;
>
> - bool enable_fbc;
> -
> bool double_wide;
>
> int pbn;
> @@ -1535,8 +1536,7 @@ static inline void
> intel_fbdev_restore_mode(struct drm_device *dev)
> #endif
>
> /* intel_fbc.c */
> -void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv,
> - struct drm_atomic_state *state);
> +void intel_fbc_check_plane(struct intel_plane_state *plane_state);
> bool intel_fbc_is_active(struct drm_i915_private *dev_priv);
> void intel_fbc_pre_update(struct intel_crtc *crtc,
> struct intel_crtc_state *crtc_state,
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c
> b/drivers/gpu/drm/i915/intel_fbc.c
> index 26a81a9..ceda6f4 100644
> --- a/drivers/gpu/drm/i915/
Re: [Intel-gfx] [PATCH 1/2] drm : adds Y-coordinate and Colorimetry Format
On Fri, 23 Dec 2016, "Nagaraju, Vathsala" wrote: > In DP V1.3 spec , Table 2-149 , page number-374 , for Register 0x2210 > , bit 7:4 is reserved. See DP 1.4. BR, Jani. > > -Original Message- > From: Jani Nikula [mailto:jani.nik...@linux.intel.com] > Sent: Friday, December 23, 2016 6:57 PM > To: Nagaraju, Vathsala ; > dri-de...@lists.freedesktop.org; intel-gfx@lists.freedesktop.org > Cc: Vivi, Rodrigo > Subject: Re: [Intel-gfx] [PATCH 1/2] drm : adds Y-coordinate and Colorimetry > Format > > On Thu, 22 Dec 2016, vathsala nagaraju wrote: >> PSR2 vsc revision number hb2( as per table 6-11)is updated to >> 4 or 5 based on Y cordinate and Colorimetry Format as below 04h = 3D >> stereo + PSR/PSR2 + Y-coordinate. >> 05h = -3D stereo- + PSR/PSR2 + Y-coordinate + Pixel >> Encoding/Colorimetry Format indication. A DP Source device is allowed >> to indicate the pixel encoding/colorimetry format to the DP Sink >> device with VSC SDP only when the DP Sink device supports it ( >> i.e.,VSC_SDP_EXTENSION_FOR_COLORIMETRY_SUPPORTED bit in the >> DPRX_FEATURE_ENUMERATION_LIST register (DPCD Address 02210h, bit 3; is >> set to 1). >> >> v2: (Jani) >> - Change DP_PSR_Y_COORDINATE to DP_PSR2_SU_Y_COORDINATE_REQUIRED. >> - Add DP_PSR2_SU_GRANULARITY_REQUIRED. >> - Change DPRX_FEATURE_ENUMERATION_LIST to DP_DPRX. >> - Add GTC_CAP and AV_SYNC_CAP, other bits in DPRX_FEATURE_ENUMERATION_LIST. >> >> Cc: Rodrigo Vivi >> Cc: Jim Bride >> Signed-off-by: Vathsala Nagaraju >> --- >> include/drm/drm_dp_helper.h | 8 +++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h >> index 55bbeb0..ee2a649d 100644 >> --- a/include/drm/drm_dp_helper.h >> +++ b/include/drm/drm_dp_helper.h >> @@ -194,7 +194,8 @@ >> # define DP_PSR_SETUP_TIME_0(6 << 1) >> # define DP_PSR_SETUP_TIME_MASK (7 << 1) >> # define DP_PSR_SETUP_TIME_SHIFT1 >> - >> +# define DP_PSR2_SU_Y_COORDINATE_REQUIRED (1 << 4) /* eDP 1.4a */ >> +# define DP_PSR2_SU_GRANULARITY_REQUIRED(1 << 5) /* eDP 1.4b */ >> /* >> * 0x80-0x8f describe downstream port capabilities, but there are two >> layouts >> * based on whether DP_DETAILED_CAP_INFO_AVAILABLE was set. If it >> was not, @@ -568,6 +569,11 @@ >> #define DP_RECEIVER_ALPM_STATUS 0x200b /* eDP 1.4 */ >> # define DP_ALPM_LOCK_TIMEOUT_ERROR (1 << 0) >> >> +#define DP_DPRX_FEATURE_ENUMERATION_LIST0x2210 >> +# define DP_GTC_CAP (1 << 0) >> +# define DP_AV_SYNC_CAP (1 << 2) >> +# define DP_VSC_SDP_EXT_FOR_COLORIMETRY_SUPPORTED (1 << 3) > > The spec continues with bits 4 to 7 in the next page... > >> + >> /* DP 1.2 Sideband message defines */ >> /* peer device type - DP 1.2a Table 2-92 */ >> #define DP_PEER_DEVICE_NONE 0x0 > > -- > Jani Nikula, Intel Open Source Technology Center -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [01/10] drm/i915: Break after walking all GGTT vma in bump_inactive_ggtt
== Series Details == Series: series starting with [01/10] drm/i915: Break after walking all GGTT vma in bump_inactive_ggtt URL : https://patchwork.freedesktop.org/series/17178/ State : success == Summary == Series 17178v1 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/17178/revisions/1/mbox/ fi-bdw-5557u total:246 pass:232 dwarn:0 dfail:0 fail:0 skip:14 fi-bsw-n3050 total:246 pass:207 dwarn:0 dfail:0 fail:0 skip:39 fi-bxt-j4205 total:246 pass:224 dwarn:0 dfail:0 fail:0 skip:22 fi-bxt-t5700 total:82 pass:69 dwarn:0 dfail:0 fail:0 skip:12 fi-byt-j1900 total:246 pass:219 dwarn:0 dfail:0 fail:0 skip:27 fi-byt-n2820 total:246 pass:215 dwarn:0 dfail:0 fail:0 skip:31 fi-hsw-4770 total:246 pass:227 dwarn:0 dfail:0 fail:0 skip:19 fi-hsw-4770r total:246 pass:227 dwarn:0 dfail:0 fail:0 skip:19 fi-ivb-3520m total:246 pass:225 dwarn:0 dfail:0 fail:0 skip:21 fi-ivb-3770 total:246 pass:225 dwarn:0 dfail:0 fail:0 skip:21 fi-kbl-7500u total:246 pass:225 dwarn:0 dfail:0 fail:0 skip:21 fi-skl-6260u total:246 pass:233 dwarn:0 dfail:0 fail:0 skip:13 fi-skl-6700hqtotal:246 pass:226 dwarn:0 dfail:0 fail:0 skip:20 fi-skl-6700k total:246 pass:222 dwarn:3 dfail:0 fail:0 skip:21 fi-skl-6770hqtotal:246 pass:233 dwarn:0 dfail:0 fail:0 skip:13 fi-snb-2600 total:246 pass:214 dwarn:0 dfail:0 fail:0 skip:32 2435e74248d49d8189c20f8d191e897460da5eb0 drm-tip: 2016y-12m-23d-14h-44m-32s UTC integration manifest 1259916 drm/i915: Prevent timeline updates whilst performing reset ab2cd91 drm/i915: Extact compute_partial_view() 1ff9e76 drm/i915: Eliminate superfluous i915_ggtt_view_normal f55cc80 drm/i915: Eliminate superfluous i915_ggtt_view_rotated a234f5c drm/i915: Convert i915_ggtt_view to use an anonymous union 0cf8be4 drm/i915: Assert that the partial VMA fits within the object 3e2b71b drm/i915: Silence allocation failure during sg_trim() a672935 drm/i915: Don't clflush before release phys object ec0f8b6 drm/i915: Repeat flush of idle work during suspend 319e26f drm/i915: Break after walking all GGTT vma in bump_inactive_ggtt == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3386/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915: enable FBC on gen9+ too (rev2)
== Series Details == Series: series starting with [1/2] drm/i915: enable FBC on gen9+ too (rev2) URL : https://patchwork.freedesktop.org/series/17176/ State : failure == Summary == Series 17176v2 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/17176/revisions/2/mbox/ Test drv_module_reload: Subgroup basic-reload: dmesg-warn -> DMESG-FAIL (fi-skl-6700k) pass -> DMESG-FAIL (fi-bxt-j4205) pass -> DMESG-FAIL (fi-kbl-7500u) pass -> DMESG-FAIL (fi-skl-6770hq) pass -> DMESG-FAIL (fi-skl-6260u) pass -> DMESG-FAIL (fi-skl-6700hq) pass -> DMESG-FAIL (fi-bxt-t5700) pass -> DMESG-FAIL (fi-bdw-5557u) Subgroup basic-reload-final: dmesg-warn -> FAIL (fi-skl-6700k) pass -> FAIL (fi-bxt-j4205) pass -> FAIL (fi-kbl-7500u) pass -> FAIL (fi-skl-6770hq) pass -> FAIL (fi-skl-6260u) pass -> FAIL (fi-skl-6700hq) pass -> FAIL (fi-bxt-t5700) pass -> FAIL (fi-bdw-5557u) Subgroup basic-reload-inject: dmesg-warn -> PASS (fi-skl-6700k) Test gem_basic: Subgroup bad-close: pass -> SKIP (fi-skl-6700k) pass -> SKIP (fi-bxt-j4205) pass -> SKIP (fi-kbl-7500u) pass -> SKIP (fi-skl-6770hq) pass -> SKIP (fi-skl-6260u) pass -> SKIP (fi-skl-6700hq) pass -> SKIP (fi-bxt-t5700) pass -> SKIP (fi-bdw-5557u) Subgroup create-close: pass -> SKIP (fi-skl-6700k) pass -> SKIP (fi-bxt-j4205) pass -> SKIP (fi-kbl-7500u) pass -> SKIP (fi-skl-6770hq) pass -> SKIP (fi-skl-6260u) pass -> SKIP (fi-skl-6700hq) pass -> SKIP (fi-bxt-t5700) pass -> SKIP (fi-bdw-5557u) Subgroup create-fd-close: pass -> SKIP (fi-skl-6700k) pass -> SKIP (fi-bxt-j4205) pass -> SKIP (fi-kbl-7500u) pass -> SKIP (fi-skl-6770hq) pass -> SKIP (fi-skl-6260u) pass -> SKIP (fi-skl-6700hq) pass -> SKIP (fi-bxt-t5700) pass -> SKIP (fi-bdw-5557u) Test gem_busy: Subgroup basic-busy-default: pass -> SKIP (fi-skl-6700k) pass -> SKIP (fi-bxt-j4205) pass -> SKIP (fi-kbl-7500u) pass -> SKIP (fi-skl-6770hq) pass -> SKIP (fi-skl-6260u) pass -> SKIP (fi-skl-6700hq) pass -> SKIP (fi-bxt-t5700) pass -> SKIP (fi-bdw-5557u) Subgroup basic-hang-default: pass -> SKIP (fi-skl-6700k) pass -> SKIP (fi-bxt-j4205) pass -> SKIP (fi-kbl-7500u) pass -> SKIP (fi-skl-6770hq) pass -> SKIP (fi-skl-6260u) pass -> SKIP (fi-skl-6700hq) pass -> SKIP (fi-bxt-t5700) pass -> SKIP (fi-bdw-5557u) Test gem_close_race: Subgroup basic-process: pass -> SKIP (fi-skl-6700k) pass -> SKIP (fi-bxt-j4205) pass -> SKIP (fi-kbl-7500u) pass -> SKIP (fi-skl-6770hq) pass -> SKIP (fi-skl-6260u) pass -> SKIP (fi-skl-6700hq) pass -> SKIP (fi-bxt-t5700) pass -> SKIP (fi-bdw-5557u) Subgroup basic-threads: pass -> SKIP (fi-skl-6700k) pass -> SKIP (fi-bxt-j4205) pass -> SKIP (fi-kbl-7500u) pass -> SKIP (fi-skl-6770hq) pass -> SKIP (fi-skl-6260u) pass -> SKIP (fi-skl-6700hq) pass -> SKIP (fi-bxt-t5700) pass -> SKIP (fi-bdw-5557u) Test gem_cpu_reloc: Subgroup basic: pass -> SKIP (fi-skl-6700k) pass -> SKIP (fi-bxt-j4205) pass
[Intel-gfx] [PATCH 1/2] drm : adds Y-coordinate and Colorimetry Format
PSR2 vsc revision number hb2( as per table 6-11)is updated to 4 or 5 based on Y cordinate and Colorimetry Format as below 04h = 3D stereo + PSR/PSR2 + Y-coordinate. 05h = -3D stereo- + PSR/PSR2 + Y-coordinate + Pixel Encoding/Colorimetry Format indication. A DP Source device is allowed to indicate the pixel encoding/colorimetry format to the DP Sink device with VSC SDP only when the DP Sink device supports it ( i.e.,VSC_SDP_EXTENSION_FOR_COLORIMETRY_SUPPORTED bit in the DPRX_FEATURE_ENUMERATION_LIST register (DPCD Address 02210h, bit 3; is set to 1). v2: (Jani) - Change DP_PSR_Y_COORDINATE to DP_PSR2_SU_Y_COORDINATE_REQUIRED. - Add DP_PSR2_SU_GRANULARITY_REQUIRED. - Change DPRX_FEATURE_ENUMERATION_LIST to DP_DPRX. - Add GTC_CAP and AV_SYNC_CAP, other bits in DPRX_FEATURE_ENUMERATION_LIST. v3: (Jani) - Add support for bits 7:4 and 1 as per DP v1.4 for DPRX_FEATURE_ENUMERATION_LIST. Cc: Rodrigo Vivi Cc: Jim Bride Signed-off-by: Vathsala Nagaraju Signed-off-by: Patil Deepti --- include/drm/drm_dp_helper.h | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 55bbeb0..0468135 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -194,7 +194,8 @@ # define DP_PSR_SETUP_TIME_0(6 << 1) # define DP_PSR_SETUP_TIME_MASK (7 << 1) # define DP_PSR_SETUP_TIME_SHIFT1 - +# define DP_PSR2_SU_Y_COORDINATE_REQUIRED (1 << 4) /* eDP 1.4a */ +# define DP_PSR2_SU_GRANULARITY_REQUIRED(1 << 5) /* eDP 1.4b */ /* * 0x80-0x8f describe downstream port capabilities, but there are two layouts * based on whether DP_DETAILED_CAP_INFO_AVAILABLE was set. If it was not, @@ -568,6 +569,16 @@ #define DP_RECEIVER_ALPM_STATUS0x200b /* eDP 1.4 */ # define DP_ALPM_LOCK_TIMEOUT_ERROR(1 << 0) +#define DP_DPRX_FEATURE_ENUMERATION_LIST0x2210 /* DP 1.3 */ +# define DP_GTC_CAP(1 << 0) /* DP 1.3 */ +# define DP_SST_SPLIT_SDP_CAP (1 << 1) /* DP 1.4 */ +# define DP_AV_SYNC_CAP(1 << 2) /* DP 1.3 */ +# define DP_VSC_SDP_EXT_FOR_COLORIMETRY_SUPPORTED (1 << 3) /* DP 1.3 */ +# define DP_VSC_EXT_VESA_SDP_SUPPORTED (1 << 4) /* DP 1.4 */ +# define DP_VSC_EXT_VESA_SDP_CHAINING_SUPPORTED(1 << 5) /* DP 1.4 */ +# define DP_VSC_EXT_CEA_SDP_SUPPORTED (1 << 6) /* DP 1.4 */ +# define DP_VSC_EXT_CEA_SDP_CHAINING_SUPPORTED (1 << 7) /* DP 1.4 */ + /* DP 1.2 Sideband message defines */ /* peer device type - DP 1.2a Table 2-92 */ #define DP_PEER_DEVICE_NONE0x0 -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct for GuC and HuC
>-Original Message-
>From: Hiler, Arkadiusz
>Sent: Friday, December 23, 2016 6:22 AM
>To: Srivatsa, Anusha
>Cc: intel-gfx@lists.freedesktop.org; Alex Dai ; Peter Antoine
>
>Subject: Re: [Intel-gfx] [PATCH 2/8] drm/i915/huc: Unified css_header struct
>for
>GuC and HuC
>
>On Thu, Dec 22, 2016 at 03:12:18PM -0800, Anusha Srivatsa wrote:
>> From: Peter Antoine
>>
>> HuC firmware css header has almost exactly same definition as GuC
>> firmware except for the sw_version. Also, add a new member fw_type
>> into intel_uc_fw to indicate what kind of fw it is. So, the loader
>> will pull right sw_version from header.
>>
>> v2: rebased on-top of drm-intel-nightly
>> v3: rebased on-top of drm-intel-nightly (again).
>> v4: rebased + spaces.
>> v7: rebased.
>> v8: rebased.
>> v9: rebased. Rename device_id to guc_branch_client_version, make
>> guc_sw_version a union. . Put UC_FW_TYPE_GUC and
>> UC_FW_TYPE_HUC into an enum.
>> v10: rebased.
>> v11: rebased.
>> v12: rebased on top of drm-tip.
>> v13: rebased.Update dev to dev_priv in intel_uc_fw_fetch
>> v14: rebased. Add INTEL_ prefix to an enum. Add fw_type declaration
>> from patch 1.Combine two different unions for huc and guc version,
>> reserved etc into one union with two structs.
>> v15: rebased.
>>
>> Tested-by: Xiang Haihao
>> Signed-off-by: Anusha Srivatsa
>> Signed-off-by: Alex Dai
>> Signed-off-by: Peter Antoine
>> ---
>> drivers/gpu/drm/i915/intel_guc_fwif.h | 23 ++
>> drivers/gpu/drm/i915/intel_guc_loader.c | 41 ++---
>
>> drivers/gpu/drm/i915/intel_uc.h | 6 +
>> 3 files changed, 53 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h
>> b/drivers/gpu/drm/i915/intel_guc_fwif.h
>> index 3202b32..ed1ab40 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
>> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
>> @@ -145,7 +145,7 @@
>> * The GuC firmware layout looks like this:
>> *
>> * +---+
>> - * |guc_css_header |
>> + * | uc_css_header |
>> * | |
>> * | contains major/minor version |
>> * +---+
>> @@ -172,9 +172,16 @@
>> * 3. Length info of each component can be found in header, in dwords.
>> * 4. Modulus and exponent key are not required by driver. They may not
>appear
>> *in fw. So driver will load a truncated firmware in this case.
>> + *
>> + * HuC firmware layout is same as GuC firmware.
>> + *
>> + * HuC firmware css header is different. However, the only difference
>> + is where
>> + * the version information is saved. The uc_css_header is unified to
>> + support
>> + * both. Driver should get HuC version from
>> + uc_css_header.huc_sw_version, while
>> + * uc_css_header.guc_sw_version for GuC.
>> */
>>
>> -struct guc_css_header {
>> +struct uc_css_header {
>> uint32_t module_type;
>> /* header_size includes all non-uCode bits, including css_header, rsa
>> * key, modulus key and exponent data. */ @@ -205,8 +212,16 @@
>> struct guc_css_header {
>>
>> char username[8];
>> char buildnumber[12];
>> -uint32_t device_id;
>> -uint32_t guc_sw_version;
>> +union {
>> +struct {
>> +uint32_t branch_client_version;
>> +uint32_t sw_version;
>> +} guc;
>> +struct {
>> +uint32_t sw_version;
>> +uint32_t reserved;
>> +} huc;
>> +};
>> uint32_t prod_preprod_fw;
>> uint32_t reserved[12];
>> uint32_t header_info;
>> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
>> b/drivers/gpu/drm/i915/intel_guc_loader.c
>> index ffe53dd7..06e3e5c 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
>> @@ -593,7 +593,7 @@ void intel_uc_fw_fetch(struct drm_i915_private
>*dev_priv,
>> struct pci_dev *pdev = dev_priv->drm.pdev;
>> struct drm_i915_gem_object *obj;
>> const struct firmware *fw = NULL;
>> -struct guc_css_header *css;
>> +struct uc_css_header *css;
>> size_t size;
>> int err;
>>
>> @@ -610,19 +610,19 @@ void intel_uc_fw_fetch(struct drm_i915_private
>*dev_priv,
>> uc_fw->uc_fw_path, fw);
>>
>> /* Check the size of the blob before examining buffer contents */
>> -if (fw->size < sizeof(struct guc_css_header)) {
>> +if (fw->size < sizeof(struct uc_css_header)) {
>> DRM_NOTE("Firmware header is missing\n");
>> goto fail;
>> }
>>
>> -css = (struct guc_css_header *)fw->data;
>> +css = (struct uc_css_header *)fw->data;
>>
>> /* Firmware bits always start from header */
>> uc_fw->header_offset = 0;
>> uc_fw->header_size = (css->header_size_dw - css->modulus_size_dw -
>> css->key_size_dw - css->exponent_size_dw) * sizeof(u32);
>>
>> -if (uc_fw->header_size != si
Re: [Intel-gfx] [GLK MIPI DSI V2 2/9] drm/i915/glk: Program new MIPI DSI PHY registers for GLK
> -Original Message-
> From: Nikula, Jani
> Sent: Friday, December 23, 2016 7:27 PM
> To: Chauhan, Madhav ; intel-
> g...@lists.freedesktop.org
> Cc: Conselvan De Oliveira, Ander ;
> Saarinen, Jani ; Konduru, Chandra
> ; Shankar, Uma ;
> Mukherjee, Indranil ; Kumar, Shobhit
> ; Deepak M ; Chauhan,
> Madhav
> Subject: Re: [GLK MIPI DSI V2 2/9] drm/i915/glk: Program new MIPI DSI PHY
> registers for GLK
>
> On Thu, 15 Dec 2016, Madhav Chauhan
> wrote:
> > From: Deepak M
> >
> > Program the clk lane and tlpx time count registers to configure DSI
> > PHY.
> >
> > v2: Addressed Jani's Review comments(renamed bit field macros)
> >
> > Signed-off-by: Deepak M
> > Signed-off-by: Madhav Chauhan
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 18 ++
> > drivers/gpu/drm/i915/intel_dsi.c | 15 +++
> > 2 files changed, 33 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h index 8e47b59..03858f9 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -8550,6 +8550,24 @@ enum {
> > #define LP_BYTECLK_SHIFT 0
> > #define LP_BYTECLK_MASK (0x << 0)
> >
> > +#define _MIPIA_TLPX_TIME_COUNT (dev_priv->mipi_mmio_base
> + 0xb0a4)
> > +#define _MIPIC_TLPX_TIME_COUNT (dev_priv->mipi_mmio_base
> + 0xb8a4)
> > +#define MIPI_TLPX_TIME_COUNT(port) _MMIO_MIPI(port,
> _MIPIA_TLPX_TIME_COUNT, _MIPIC_TLPX_TIME_COUNT)
> > +#define GLK_DPHY_TLPX_TIME_CNT_SHIFT 0
> > +#define GLK_DPHY_TLPX_TIME_CNT_MASK (0xff << 0)
> > +
> > +#define _MIPIA_CLK_LANE_TIMING (dev_priv->mipi_mmio_base
> + 0xb098)
> > +#define _MIPIC_CLK_LANE_TIMING (dev_priv->mipi_mmio_base
> + 0xb898)
> > +#define MIPI_CLK_LANE_TIMING(port) _MMIO_MIPI(port,
> _MIPIA_CLK_LANE_TIMING, _MIPIC_CLK_LANE_TIMING)
> > +#define GLK_MIPI_CLK_LANE_HS_PREP_SHIFT 0
> > +#define GLK_MIPI_CLK_LANE_HS_PREP_MASK(0xff
> << 0)
> > +#define GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT 8
> > +#define GLK_MIPI_CLK_LANE_HS_ZERO_MASK
> (0xff00 << 0)
>
> 0xff << 8
>
> > +#define GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT 16
> > +#define GLK_MIPI_CLK_LANE_HS_TRAIL_MASK
> (0xff << 0)
>
> 0xff << 16
>
> > +#define GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT 24
> > +#define GLK_MIPI_CLK_LANE_HS_EXIT_MASK
> (0xff00 << 0)
>
> 0xff << 24
>
> > +
> > /* bits 31:0 */
> > #define _MIPIA_LP_GEN_DATA (dev_priv->mipi_mmio_base
> + 0xb064)
> > #define _MIPIC_LP_GEN_DATA (dev_priv->mipi_mmio_base
> + 0xb864)
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> > b/drivers/gpu/drm/i915/intel_dsi.c
> > index 6b63355..b78c686 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -1123,6 +1123,7 @@ static void intel_dsi_prepare(struct intel_encoder
> *intel_encoder,
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
> > struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
> > + struct mipi_config *mipi_config = dev_priv->vbt.dsi.config;
> > const struct drm_display_mode *adjusted_mode = &pipe_config-
> >base.adjusted_mode;
> > enum port port;
> > unsigned int bpp =
> > mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
> > @@ -1278,6 +1279,20 @@ static void intel_dsi_prepare(struct
> intel_encoder *intel_encoder,
> > */
> > I915_WRITE(MIPI_LP_BYTECLK(port), intel_dsi->lp_byte_clk);
> >
> > + if (IS_GEMINILAKE(dev_priv)) {
> > + I915_WRITE(MIPI_TLPX_TIME_COUNT(port),
> > + intel_dsi->lp_byte_clk);
> > + val = ((mipi_config->ths_prepare <<
> > + GLK_MIPI_CLK_LANE_HS_PREP_SHIFT) |
> > + (mipi_config->ths_prepare_hszero <<
> > + GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT) |
> > + (mipi_config->ths_trail <<
> > + GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT) |
> > + (mipi_config->ths_exit <<
> > + GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT));
> > + I915_WRITE(MIPI_CLK_LANE_TIMING(port), val);
> > + }
>
> Please fix this as Ville suggested, i.e. don't look at mipi_config directly in
> intel_dsi.c. See what gets done with dphy_reg.
>
> We may change this later, but for now I think this is the right thing to do.
Thanks. Will send the patch in next series, early next week.
>
> BR,
> Jani.
>
> > +
> > /* the bw essential for transmitting 16 long packets
> containing
> > * 252 bytes meant for dcs write memory command is
> programmed in
> > * this register in te
Re: [Intel-gfx] [PATCH 4/5] drm/i915/guc: Extract param logic form guc_init
On 15/12/16 07:47, Arkadiusz Hiler wrote:
Let intel_guc_init() focus on determining and fetching the correct
firmware.
This patch introduces intel_sanitize_uc_params() that is called from
intel_uc_init().
Then, if we have GuC, we can call intel_guc_init() conditionally and we
do not have to do internal checks.
It can be easily extended to support HuC case as well.
Cc: Anusha Srivatsa
Cc: Jeff McGee
Cc: Michal Winiarski
Signed-off-by: Arkadiusz Hiler
---
drivers/gpu/drm/i915/intel_guc_loader.c | 19 +--
drivers/gpu/drm/i915/intel_uc.c | 23 ++-
2 files changed, 23 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
b/drivers/gpu/drm/i915/intel_guc_loader.c
index b76b556..0bb5fd1 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -617,7 +617,7 @@ static void guc_fw_fetch(struct drm_i915_private *dev_priv,
}
/**
- * intel_guc_init() - define parameters and fetch firmware
+ * intel_guc_init() - determine and fetch firmware
* @dev_priv: i915 device private
*
* Called early during driver load, but after GEM is initialised.
@@ -630,17 +630,6 @@ void intel_guc_init(struct drm_i915_private *dev_priv)
struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
const char *fw_path;
- if (!HAS_GUC(dev_priv)) {
- i915.enable_guc_loading = 0;
- i915.enable_guc_submission = 0;
- } else {
- /* A negative value means "use platform default" */
- if (i915.enable_guc_loading < 0)
- i915.enable_guc_loading = HAS_GUC_UCODE(dev_priv);
- if (i915.enable_guc_submission < 0)
- i915.enable_guc_submission = HAS_GUC_SCHED(dev_priv);
- }
-
if (!HAS_GUC_UCODE(dev_priv)) {
fw_path = NULL;
} else if (IS_SKYLAKE(dev_priv)) {
@@ -663,13 +652,7 @@ void intel_guc_init(struct drm_i915_private *dev_priv)
guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_NONE;
guc_fw->guc_fw_load_status = GUC_FIRMWARE_NONE;
- /* can't enable guc submission without guc */
- if (!i915.enable_guc_loading)
- i915.enable_guc_submission = 0;
-
/* Early (and silent) return if GuC loading is disabled */
- if (!i915.enable_guc_loading)
- return;
if (fw_path == NULL)
return;
if (*fw_path == '\0')
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 4e184edb..e72f784 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -25,6 +25,24 @@
#include "i915_drv.h"
#include "intel_uc.h"
+static void intel_sanitize_uc_params(struct drm_i915_private *dev_priv)
+{
+ if (!HAS_GUC(dev_priv)) {
+ i915.enable_guc_loading = 0;
+ i915.enable_guc_submission = 0;
+ } else {
+ /* A negative value means "use platform default" */
+ if (i915.enable_guc_loading < 0)
+ i915.enable_guc_loading = HAS_GUC_UCODE(dev_priv);
+ if (i915.enable_guc_submission < 0)
+ i915.enable_guc_submission = HAS_GUC_SCHED(dev_priv);
+ }
+
+ /* can't enable guc submission without guc */
+ if (!i915.enable_guc_loading)
+ i915.enable_guc_submission = 0;
+}
+
void intel_uc_init_early(struct drm_i915_private *dev_priv)
{
mutex_init(&dev_priv->guc.send_mutex);
@@ -32,7 +50,10 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv)
void intel_uc_init(struct drm_i915_private *dev_priv)
{
- intel_guc_init(dev_priv);
+ intel_sanitize_uc_params(dev_priv);
If we place intel_sanitize_uc_params in intel_sanitize_options we can
have the parameters resolved earlier and we should therefore be able to
use them to better handle GuC-related differences during driver load.
Daniele
+
+ if (i915.enable_guc_loading)
+ intel_guc_init(dev_priv);
}
int intel_uc_load(struct drm_i915_private *dev_priv)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 1/2] drm: Wrap the check for atomic_commit implementation
This check is useful for drivers that do not have DRIVER_ATOMIC set but
have atomic modesetting internally implemented. Wrap the check into a
function since this is used in many places and as a bonus, the function
name helps to document what the check is for.
v2:
Change return type to bool (Ville)
Move the function drm_atomic.h (Daniel)
Fixed comment marker for documentation
v3:
Included drmP.h to dereference struct drm_device
Suggested-by: Daniel Vetter
Cc: Daniel Vetter
Cc: Ben Skeggs
Signed-off-by: Dhinakaran Pandiyan
---
drivers/gpu/drm/drm_fb_helper.c | 6 +++---
drivers/gpu/drm/nouveau/nouveau_connector.c | 5 +++--
drivers/gpu/drm/nouveau/nouveau_display.c | 6 +++---
drivers/gpu/drm/nouveau/nouveau_fbcon.c | 3 ++-
include/drm/drm_atomic.h| 13 +
5 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 145d55f..730342c 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -405,7 +405,7 @@ static int restore_fbdev_mode(struct drm_fb_helper
*fb_helper)
drm_warn_on_modeset_not_all_locked(dev);
- if (dev->mode_config.funcs->atomic_commit)
+ if (drm_drv_uses_atomic_modeset(dev))
return restore_fbdev_mode_atomic(fb_helper);
drm_for_each_plane(plane, dev) {
@@ -1444,7 +1444,7 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo
*var,
return -EBUSY;
}
- if (dev->mode_config.funcs->atomic_commit) {
+ if (drm_drv_uses_atomic_modeset(dev)) {
ret = pan_display_atomic(var, info);
goto unlock;
}
@@ -2060,7 +2060,7 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
* NULL we fallback to the default drm_atomic_helper_best_encoder()
* helper.
*/
- if (fb_helper->dev->mode_config.funcs->atomic_commit &&
+ if (drm_drv_uses_atomic_modeset(fb_helper->dev) &&
!connector_funcs->best_encoder)
encoder = drm_atomic_helper_best_encoder(connector);
else
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c
b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 947c200..966d20a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -33,6 +33,7 @@
#include
#include
#include
+#include
#include "nouveau_reg.h"
#include "nouveau_drv.h"
@@ -769,7 +770,7 @@ nouveau_connector_set_property(struct drm_connector
*connector,
struct drm_encoder *encoder = to_drm_encoder(nv_encoder);
int ret;
- if (connector->dev->mode_config.funcs->atomic_commit)
+ if (drm_drv_uses_atomic_modeset(connector->dev))
return drm_atomic_helper_connector_set_property(connector,
property, value);
ret = connector->funcs->atomic_set_property(&nv_connector->base,
@@ -1074,7 +1075,7 @@ nouveau_connector_helper_funcs = {
static int
nouveau_connector_dpms(struct drm_connector *connector, int mode)
{
- if (connector->dev->mode_config.funcs->atomic_commit)
+ if (drm_drv_uses_atomic_modeset(connector->dev))
return drm_atomic_helper_connector_dpms(connector, mode);
return drm_helper_connector_dpms(connector, mode);
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c
b/drivers/gpu/drm/nouveau/nouveau_display.c
index c5cf888..add353e 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -162,7 +162,7 @@ nouveau_display_vblstamp(struct drm_device *dev, unsigned
int pipe,
list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
if (nouveau_crtc(crtc)->index == pipe) {
struct drm_display_mode *mode;
- if (dev->mode_config.funcs->atomic_commit)
+ if (drm_drv_uses_atomic_modeset(dev))
mode = &crtc->state->adjusted_mode;
else
mode = &crtc->hwmode;
@@ -738,7 +738,7 @@ nouveau_display_suspend(struct drm_device *dev, bool
runtime)
struct nouveau_display *disp = nouveau_display(dev);
struct drm_crtc *crtc;
- if (dev->mode_config.funcs->atomic_commit) {
+ if (drm_drv_uses_atomic_modeset(dev)) {
if (!runtime) {
disp->suspend = nouveau_atomic_suspend(dev);
if (IS_ERR(disp->suspend)) {
@@ -784,7 +784,7 @@ nouveau_display_resume(struct drm_device *dev, bool runtime)
struct drm_crtc *crtc;
int ret;
- if (dev->mode_config.funcs->atomic_commit) {
+ if (drm_drv_uses_atomic_modeset(dev)) {
nouveau_display_init(dev);
if (disp->suspend) {
drm_atomic_helper_resume(dev, disp->suspend);
diff --git a/drivers/gpu/drm/nouveau/nouveau_fb
[Intel-gfx] [PATCH v4 2/2] drm: Get atomic property value even if DRIVER_ATOMIC is not set
i915 does not set DRIVER_ATOMIC by default yet but uses atomic_check and atomic_commit. drm_object_property_get_value() does not read the correct value of atomic properties if DRIVER_ATOMIC is not set. Checking whether the driver uses atomic modeset is a better check instead as the property values are tracked in the state structures. v2: Included header Cc: Daniel Vetter Reviewed-by: Daniel Vetter Signed-off-by: Dhinakaran Pandiyan --- drivers/gpu/drm/drm_mode_object.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c index 9f17085..14543ff 100644 --- a/drivers/gpu/drm/drm_mode_object.c +++ b/drivers/gpu/drm/drm_mode_object.c @@ -23,6 +23,7 @@ #include #include #include +#include #include "drm_crtc_internal.h" @@ -273,7 +274,7 @@ int drm_object_property_get_value(struct drm_mode_object *obj, * their value in obj->properties->values[].. mostly to avoid * having to deal w/ EDID and similar props in atomic paths: */ - if (drm_core_check_feature(property->dev, DRIVER_ATOMIC) && + if (drm_drv_uses_atomic_modeset(property->dev) && !(property->flags & DRM_MODE_PROP_IMMUTABLE)) return drm_atomic_get_property(obj, property, val); -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v4,1/2] drm: Wrap the check for atomic_commit implementation
== Series Details == Series: series starting with [v4,1/2] drm: Wrap the check for atomic_commit implementation URL : https://patchwork.freedesktop.org/series/17193/ State : success == Summary == Series 17193v1 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/17193/revisions/1/mbox/ fi-bdw-5557u total:246 pass:232 dwarn:0 dfail:0 fail:0 skip:14 fi-bsw-n3050 total:246 pass:207 dwarn:0 dfail:0 fail:0 skip:39 fi-bxt-j4205 total:246 pass:224 dwarn:0 dfail:0 fail:0 skip:22 fi-bxt-t5700 total:82 pass:69 dwarn:0 dfail:0 fail:0 skip:12 fi-byt-j1900 total:246 pass:219 dwarn:0 dfail:0 fail:0 skip:27 fi-byt-n2820 total:246 pass:215 dwarn:0 dfail:0 fail:0 skip:31 fi-hsw-4770 total:246 pass:227 dwarn:0 dfail:0 fail:0 skip:19 fi-hsw-4770r total:246 pass:227 dwarn:0 dfail:0 fail:0 skip:19 fi-ivb-3520m total:246 pass:225 dwarn:0 dfail:0 fail:0 skip:21 fi-ivb-3770 total:246 pass:225 dwarn:0 dfail:0 fail:0 skip:21 fi-kbl-7500u total:246 pass:225 dwarn:0 dfail:0 fail:0 skip:21 fi-skl-6260u total:246 pass:233 dwarn:0 dfail:0 fail:0 skip:13 fi-skl-6700hqtotal:246 pass:226 dwarn:0 dfail:0 fail:0 skip:20 fi-skl-6700k total:246 pass:222 dwarn:3 dfail:0 fail:0 skip:21 fi-skl-6770hqtotal:246 pass:233 dwarn:0 dfail:0 fail:0 skip:13 fi-snb-2520m total:246 pass:215 dwarn:0 dfail:0 fail:0 skip:31 fi-snb-2600 total:246 pass:214 dwarn:0 dfail:0 fail:0 skip:32 facc8fa36acbf8244b601a0eec21e6d030061a3f drm-tip: 2016y-12m-23d-16h-09m-03s UTC integration manifest 20ede2d drm: Get atomic property value even if DRIVER_ATOMIC is not set 0903b0c drm: Wrap the check for atomic_commit implementation == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3388/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 1/2] drm/i915: request ring to be pinned above GUC_WOPCM_TOP
From: Daniele Ceraolo Spurio
GuC will validate the ring offset and fail if it is in the
[0, GUC_WOPCM_TOP) range. The bias is conditionally applied only
if GuC loading is enabled (we can't check for guc submission enabled as
in other cases because HuC loading requires this fix).
Note that the default context is processed before enable_guc_loading is
sanitized, so we might still apply the bias to its ring even if it is
not needed.
v2: compute the value during ctx init and pass it to
intel_ring_pin (Chris), updated commit message
Signed-off-by: Daniele Ceraolo Spurio
Cc: Chris Wilson
Cc: Michal Wajdeczko
Cc: Arkadiusz Hiler
Cc: Anusha Srivatsa
Cc: Michał Winiarski
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem_context.c | 9 +
drivers/gpu/drm/i915/intel_lrc.c| 2 +-
drivers/gpu/drm/i915/intel_ringbuffer.c | 11 +++
drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +-
5 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ad4b9a1..41e939f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1087,6 +1087,7 @@ struct i915_gem_context {
int priority; /* greater priorities are serviced first */
u32 ggtt_alignment;
+ u32 ggtt_offset_bias;
struct intel_context {
struct i915_vma *state;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index 15b25c1..48e4ed5 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -335,6 +335,15 @@ static int assign_hw_id(struct drm_i915_private *dev_priv,
unsigned *out)
GEN8_CTX_ADDRESSING_MODE_SHIFT;
ATOMIC_INIT_NOTIFIER_HEAD(&ctx->status_notifier);
+ /* GuC requires the ring to be placed above GUC_WOPCM_TOP. If GuC is not
+* present or not in use we still need a small bias as ring wraparound
+* at offset 0 sometimes hangs. No idea why.
+*/
+ if (HAS_GUC(dev_priv) && i915.enable_guc_loading)
+ ctx->ggtt_offset_bias = GUC_WOPCM_TOP;
+ else
+ ctx->ggtt_offset_bias = 4096;
+
return ctx;
err_pid:
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index d322d3c..cec9037 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -796,7 +796,7 @@ static int execlists_context_pin(struct intel_engine_cs
*engine,
goto unpin_vma;
}
- ret = intel_ring_pin(ce->ring);
+ ret = intel_ring_pin(ce->ring, ctx->ggtt_offset_bias);
if (ret)
goto unpin_map;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 69ccf4f..d0073f0 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1805,10 +1805,9 @@ static int init_phys_status_page(struct intel_engine_cs
*engine)
return 0;
}
-int intel_ring_pin(struct intel_ring *ring)
+int intel_ring_pin(struct intel_ring *ring, unsigned int offset_bias)
{
- /* Ring wraparound at offset 0 sometimes hangs. No idea why. */
- unsigned int flags = PIN_GLOBAL | PIN_OFFSET_BIAS | 4096;
+ unsigned int flags = PIN_GLOBAL;
enum i915_map_type map;
struct i915_vma *vma = ring->vma;
void *addr;
@@ -1818,6 +1817,9 @@ int intel_ring_pin(struct intel_ring *ring)
map = HAS_LLC(ring->engine->i915) ? I915_MAP_WB : I915_MAP_WC;
+ if (offset_bias)
+ flags |= PIN_OFFSET_BIAS | offset_bias;
+
if (vma->obj->stolen)
flags |= PIN_MAPPABLE;
@@ -2046,7 +2048,8 @@ static int intel_init_ring_buffer(struct intel_engine_cs
*engine)
goto error;
}
- ret = intel_ring_pin(ring);
+ /* Ring wraparound at offset 0 sometimes hangs. No idea why. */
+ ret = intel_ring_pin(ring, 4096);
if (ret) {
intel_ring_free(ring);
goto error;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 0969de7..79c2b8d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -483,7 +483,7 @@ struct intel_engine_cs {
struct intel_ring *
intel_engine_create_ring(struct intel_engine_cs *engine, int size);
-int intel_ring_pin(struct intel_ring *ring);
+int intel_ring_pin(struct intel_ring *ring, unsigned int offset_bias);
void intel_ring_unpin(struct intel_ring *ring);
void intel_ring_free(struct intel_ring *ring);
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: re-use computed offset bias for context pin
From: Daniele Ceraolo Spurio
The context has to obey the same offset requirements as the ring,
so we can re-use the same bias value we computed for the ring instead of
unconditionally using GUC_WOPCM_TOP.
Suggested-by: Chris Wilson
Signed-off-by: Daniele Ceraolo Spurio
Cc: Chris Wilson
---
drivers/gpu/drm/i915/intel_lrc.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index cec9037..0a321d1 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -767,7 +767,7 @@ static int execlists_context_pin(struct intel_engine_cs
*engine,
struct i915_gem_context *ctx)
{
struct intel_context *ce = &ctx->engine[engine->id];
- unsigned int flags;
+ unsigned int flags = PIN_GLOBAL;
void *vaddr;
int ret;
@@ -782,7 +782,9 @@ static int execlists_context_pin(struct intel_engine_cs
*engine,
goto err;
}
- flags = PIN_OFFSET_BIAS | GUC_WOPCM_TOP | PIN_GLOBAL;
+ if (ctx->ggtt_offset_bias)
+ flags |= PIN_OFFSET_BIAS | ctx->ggtt_offset_bias;
+
if (ctx == ctx->i915->kernel_context)
flags |= PIN_HIGH;
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v2,1/2] drm/i915: request ring to be pinned above GUC_WOPCM_TOP
== Series Details == Series: series starting with [v2,1/2] drm/i915: request ring to be pinned above GUC_WOPCM_TOP URL : https://patchwork.freedesktop.org/series/17195/ State : success == Summary == Series 17195v1 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/17195/revisions/1/mbox/ fi-bdw-5557u total:246 pass:232 dwarn:0 dfail:0 fail:0 skip:14 fi-bsw-n3050 total:246 pass:207 dwarn:0 dfail:0 fail:0 skip:39 fi-bxt-j4205 total:246 pass:224 dwarn:0 dfail:0 fail:0 skip:22 fi-bxt-t5700 total:82 pass:69 dwarn:0 dfail:0 fail:0 skip:12 fi-byt-j1900 total:246 pass:219 dwarn:0 dfail:0 fail:0 skip:27 fi-byt-n2820 total:246 pass:215 dwarn:0 dfail:0 fail:0 skip:31 fi-hsw-4770 total:246 pass:227 dwarn:0 dfail:0 fail:0 skip:19 fi-hsw-4770r total:246 pass:227 dwarn:0 dfail:0 fail:0 skip:19 fi-ivb-3520m total:246 pass:225 dwarn:0 dfail:0 fail:0 skip:21 fi-ivb-3770 total:246 pass:225 dwarn:0 dfail:0 fail:0 skip:21 fi-kbl-7500u total:246 pass:225 dwarn:0 dfail:0 fail:0 skip:21 fi-skl-6260u total:246 pass:233 dwarn:0 dfail:0 fail:0 skip:13 fi-skl-6700hqtotal:246 pass:226 dwarn:0 dfail:0 fail:0 skip:20 fi-skl-6700k total:246 pass:222 dwarn:3 dfail:0 fail:0 skip:21 fi-skl-6770hqtotal:246 pass:233 dwarn:0 dfail:0 fail:0 skip:13 fi-snb-2520m total:246 pass:215 dwarn:0 dfail:0 fail:0 skip:31 fi-snb-2600 total:246 pass:214 dwarn:0 dfail:0 fail:0 skip:32 facc8fa36acbf8244b601a0eec21e6d030061a3f drm-tip: 2016y-12m-23d-16h-09m-03s UTC integration manifest 32cb6ef drm/i915: re-use computed offset bias for context pin 13f69d8 drm/i915: request ring to be pinned above GUC_WOPCM_TOP == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3389/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
