* [PATCH v2 3/7] rslib: decode_rs: Fix length parameter check
From: Ferdinand Blomqvist @ 2019-06-20 14:10 UTC (permalink / raw)
To: linux-kernel; +Cc: Thomas Gleixner
In-Reply-To: <20190620141039.9874-1-ferdinand.blomqvist@gmail.com>
The length of the data load must be at least one. Or in other words,
there must be room for at least 1 data and nroots parity symbols after
shortening the RS code.
Signed-off-by: Ferdinand Blomqvist <ferdinand.blomqvist@gmail.com>
---
lib/reed_solomon/decode_rs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/reed_solomon/decode_rs.c b/lib/reed_solomon/decode_rs.c
index 3313bf944ff1..22006eaa41e6 100644
--- a/lib/reed_solomon/decode_rs.c
+++ b/lib/reed_solomon/decode_rs.c
@@ -39,7 +39,7 @@
/* Check length parameter for validity */
pad = nn - nroots - len;
- BUG_ON(pad < 0 || pad >= nn);
+ BUG_ON(pad < 0 || pad >= nn - nroots);
/* Does the caller provide the syndrome ? */
if (s != NULL)
--
2.17.2
^ permalink raw reply related
* [PATCH v2 2/7] rslib: Fix decoding of shortened codes
From: Ferdinand Blomqvist @ 2019-06-20 14:10 UTC (permalink / raw)
To: linux-kernel; +Cc: Thomas Gleixner
In-Reply-To: <20190620141039.9874-1-ferdinand.blomqvist@gmail.com>
The decoding of shortenend codes is broken. It only works as expected if
there are no erasures.
When decoding with erasures, Lambda (the error and erasure locator
polynomial) is initialized from the given erasure positions. The pad
parameter is not accounted for by the initialisation code, and hence
Lambda is initialized from incorrect erasure positions. The fix is
to adjust the erasure positions by the supplied pad.
Signed-off-by: Ferdinand Blomqvist <ferdinand.blomqvist@gmail.com>
---
lib/reed_solomon/decode_rs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/reed_solomon/decode_rs.c b/lib/reed_solomon/decode_rs.c
index 1db74eb098d0..3313bf944ff1 100644
--- a/lib/reed_solomon/decode_rs.c
+++ b/lib/reed_solomon/decode_rs.c
@@ -99,9 +99,9 @@
if (no_eras > 0) {
/* Init lambda to be the erasure locator polynomial */
lambda[1] = alpha_to[rs_modnn(rs,
- prim * (nn - 1 - eras_pos[0]))];
+ prim * (nn - 1 - (eras_pos[0] + pad)))];
for (i = 1; i < no_eras; i++) {
- u = rs_modnn(rs, prim * (nn - 1 - eras_pos[i]));
+ u = rs_modnn(rs, prim * (nn - 1 - (eras_pos[i] + pad)));
for (j = i + 1; j > 0; j--) {
tmp = index_of[lambda[j - 1]];
if (tmp != nn) {
--
2.17.2
^ permalink raw reply related
* [PATCH v2 7/7] rslib: Fix remaining decoder flaws
From: Ferdinand Blomqvist @ 2019-06-20 14:10 UTC (permalink / raw)
To: linux-kernel; +Cc: Thomas Gleixner
In-Reply-To: <20190620141039.9874-1-ferdinand.blomqvist@gmail.com>
The decoder is flawed in the following ways:
- The decoder sometimes fails silently, i.e. it announces success but
returns a word that is not a codeword.
- The return value of the decoder is incoherent with respect to how
fixed erasures are counted. If the word to be decoded is a codeword,
then the decoder always returns zero even if some erasures are given.
On the other hand, if the word to be decoded contains errors, then the
number of erasures is always included in the count of corrected
symbols. So the decoder handles erasures without symbol corruption
inconsistently. This inconsistency probably doesn't affect anyone
using the decoder, but it is inconsistent with the documentation.
- The error positions returned in eras_pos include all erasures, but the
corrections are only set in the correction buffer if there actually is
a symbol error. So if there are erasures without symbol corruption,
then the correction buffer will contain errors (unless initialized to
zero before calling the decoder) or some values will be unset (if the
correction buffer is uninitialized).
- When correcting data in-place the decoder does not correct errors in
the parity. On the other hand, when returning the errors in correction
buffers, errors in the parity are included.
The respective fixed are:
- The syndrome of a codeword is always zero, and the syndrome is linear,
.i.e, S(x+e) = S(x) + S(e). So compute the syndrome for the error and
check whether it equals the syndrome of the received word. If it does,
then we have decoded to a valid codeword, otherwise we know that we
have an uncorrectable error. Fortunately, some unrecoverable error
conditions can be detected earlier in the decoding, which saves some
processing power.
- Simply count and return the number of symbols actually corrected.
- Make sure to only return positions where symbols were corrected.
- Also fix errors in parity when correcting in-place. Another option
would be to completely disregard errors in the parity, but then the
interface makes it impossible to write tests that test for silent
failures.
Other changes:
- Only fill the correction buffer and error position buffer if both of
them are provided. Otherwise correct in place. Previously the error
position buffer was always populated with the positions of the
corrected errors, irrespective of whether a correction buffer was
supplied or not. The rationale for this change is that there seems to
be two use cases for the decoder; correct in-place or use the
correction buffers. The caller does not need the positions of the
corrected errors when in-place correction is used. If in-place
correction is not used, then both the correction buffer and error
position buffer need to be populated.
Signed-off-by: Ferdinand Blomqvist <ferdinand.blomqvist@gmail.com>
---
lib/reed_solomon/decode_rs.c | 88 ++++++++++++++++++++++++++++--------
1 file changed, 68 insertions(+), 20 deletions(-)
diff --git a/lib/reed_solomon/decode_rs.c b/lib/reed_solomon/decode_rs.c
index b7264a712d46..805de84ae83d 100644
--- a/lib/reed_solomon/decode_rs.c
+++ b/lib/reed_solomon/decode_rs.c
@@ -22,6 +22,7 @@
uint16_t *index_of = rs->index_of;
uint16_t u, q, tmp, num1, num2, den, discr_r, syn_error;
int count = 0;
+ int num_corrected;
uint16_t msk = (uint16_t) rs->nn;
/*
@@ -184,6 +185,15 @@
if (lambda[i] != nn)
deg_lambda = i;
}
+
+ if (deg_lambda == 0) {
+ /*
+ * deg(lambda) is zero even though the syndrome is non-zero
+ * => uncorrectable error detected
+ */
+ return -EBADMSG;
+ }
+
/* Find roots of error+erasure locator polynomial by Chien search */
memcpy(®[1], &lambda[1], nroots * sizeof(reg[0]));
count = 0; /* Number of roots of lambda(x) */
@@ -197,6 +207,12 @@
}
if (q != 0)
continue; /* Not a root */
+
+ if (k < pad) {
+ /* Impossible error location. Uncorrectable error. */
+ return -EBADMSG;
+ }
+
/* store root (index-form) and error location number */
root[count] = i;
loc[count] = k;
@@ -231,7 +247,9 @@
/*
* Compute error values in poly-form. num1 = omega(inv(X(l))), num2 =
* inv(X(l))**(fcr-1) and den = lambda_pr(inv(X(l))) all in poly-form
+ * Note: we reuse the buffer for b to store the correction pattern
*/
+ num_corrected = 0;
for (j = count - 1; j >= 0; j--) {
num1 = 0;
for (i = deg_omega; i >= 0; i--) {
@@ -239,6 +257,13 @@
num1 ^= alpha_to[rs_modnn(rs, omega[i] +
i * root[j])];
}
+
+ if (num1 == 0) {
+ /* Nothing to correct at this position */
+ b[j] = 0;
+ continue;
+ }
+
num2 = alpha_to[rs_modnn(rs, root[j] * (fcr - 1) + nn)];
den = 0;
@@ -250,29 +275,52 @@
i * root[j])];
}
}
- /* Apply error to data */
- if (num1 != 0 && loc[j] >= pad) {
- uint16_t cor = alpha_to[rs_modnn(rs,index_of[num1] +
- index_of[num2] +
- nn - index_of[den])];
- /* Store the error correction pattern, if a
- * correction buffer is available */
- if (corr) {
- corr[j] = cor;
- } else {
- /* If a data buffer is given and the
- * error is inside the message,
- * correct it */
- if (data && (loc[j] < (nn - nroots)))
- data[loc[j] - pad] ^= cor;
- }
+
+ b[j] = alpha_to[rs_modnn(rs, index_of[num1] +
+ index_of[num2] +
+ nn - index_of[den])];
+ num_corrected++;
+ }
+
+ /*
+ * We compute the syndrome of the 'error' and check that it matches
+ * the syndrome of the received word
+ */
+ for (i = 0; i < nroots; i++) {
+ tmp = 0;
+ for (j = 0; j < count; j++) {
+ if (b[j] == 0)
+ continue;
+
+ k = (fcr + i) * prim * (nn-loc[j]-1);
+ tmp ^= alpha_to[rs_modnn(rs, index_of[b[j]] + k)];
}
+
+ if (tmp != alpha_to[s[i]])
+ return -EBADMSG;
}
- if (eras_pos != NULL) {
- for (i = 0; i < count; i++)
- eras_pos[i] = loc[i] - pad;
+ /*
+ * Store the error correction pattern, if a
+ * correction buffer is available
+ */
+ if (corr && eras_pos) {
+ j = 0;
+ for (i = 0; i < count; i++) {
+ if (b[i]) {
+ corr[j] = b[i];
+ eras_pos[j++] = loc[i] - pad;
+ }
+ }
+ } else if (data && par) {
+ /* Apply error to data and parity */
+ for (i = 0; i < count; i++) {
+ if (loc[i] < (nn - nroots))
+ data[loc[i] - pad] ^= b[i];
+ else
+ par[loc[i] - pad - len] ^= b[i];
+ }
}
- return count;
+ return num_corrected;
}
--
2.17.2
^ permalink raw reply related
* [PATCH v2 6/7] rslib: Update documentation
From: Ferdinand Blomqvist @ 2019-06-20 14:10 UTC (permalink / raw)
To: linux-kernel; +Cc: Thomas Gleixner
In-Reply-To: <20190620141039.9874-1-ferdinand.blomqvist@gmail.com>
The decoder returns the number of corrected symbols, not bits.
The caller provided syndrome must be in index form.
Signed-off-by: Ferdinand Blomqvist <ferdinand.blomqvist@gmail.com>
---
lib/reed_solomon/reed_solomon.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/lib/reed_solomon/reed_solomon.c b/lib/reed_solomon/reed_solomon.c
index e5fdc8b9e856..bbc01bad3053 100644
--- a/lib/reed_solomon/reed_solomon.c
+++ b/lib/reed_solomon/reed_solomon.c
@@ -340,7 +340,8 @@ EXPORT_SYMBOL_GPL(encode_rs8);
* @data: data field of a given type
* @par: received parity data field
* @len: data length
- * @s: syndrome data field (if NULL, syndrome is calculated)
+ * @s: syndrome data field, must be in index form
+ * (if NULL, syndrome is calculated)
* @no_eras: number of erasures
* @eras_pos: position of erasures, can be NULL
* @invmsk: invert data mask (will be xored on data, not on parity!)
@@ -354,7 +355,8 @@ EXPORT_SYMBOL_GPL(encode_rs8);
* decoding, so the caller has to ensure that decoder invocations are
* serialized.
*
- * Returns the number of corrected bits or -EBADMSG for uncorrectable errors.
+ * Returns the number of corrected symbols or -EBADMSG for uncorrectable
+ * errors. The count includes errors in the parity.
*/
int decode_rs8(struct rs_control *rsc, uint8_t *data, uint16_t *par, int len,
uint16_t *s, int no_eras, int *eras_pos, uint16_t invmsk,
@@ -391,7 +393,8 @@ EXPORT_SYMBOL_GPL(encode_rs16);
* @data: data field of a given type
* @par: received parity data field
* @len: data length
- * @s: syndrome data field (if NULL, syndrome is calculated)
+ * @s: syndrome data field, must be in index form
+ * (if NULL, syndrome is calculated)
* @no_eras: number of erasures
* @eras_pos: position of erasures, can be NULL
* @invmsk: invert data mask (will be xored on data, not on parity!)
@@ -403,7 +406,8 @@ EXPORT_SYMBOL_GPL(encode_rs16);
* decoding, so the caller has to ensure that decoder invocations are
* serialized.
*
- * Returns the number of corrected bits or -EBADMSG for uncorrectable errors.
+ * Returns the number of corrected symbols or -EBADMSG for uncorrectable
+ * errors. The count includes errors in the parity.
*/
int decode_rs16(struct rs_control *rsc, uint16_t *data, uint16_t *par, int len,
uint16_t *s, int no_eras, int *eras_pos, uint16_t invmsk,
--
2.17.2
^ permalink raw reply related
* [PATCH v2 5/7] rslib: Fix handling of of caller provided syndrome
From: Ferdinand Blomqvist @ 2019-06-20 14:10 UTC (permalink / raw)
To: linux-kernel; +Cc: Thomas Gleixner
In-Reply-To: <20190620141039.9874-1-ferdinand.blomqvist@gmail.com>
Check if the syndrome provided by the caller is zero, and act
accordingly.
Signed-off-by: Ferdinand Blomqvist <ferdinand.blomqvist@gmail.com>
---
lib/reed_solomon/decode_rs.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/lib/reed_solomon/decode_rs.c b/lib/reed_solomon/decode_rs.c
index 78629bbe6590..b7264a712d46 100644
--- a/lib/reed_solomon/decode_rs.c
+++ b/lib/reed_solomon/decode_rs.c
@@ -42,8 +42,18 @@
BUG_ON(pad < 0 || pad >= nn - nroots);
/* Does the caller provide the syndrome ? */
- if (s != NULL)
- goto decode;
+ if (s != NULL) {
+ for (i = 0; i < nroots; i++) {
+ /* The syndrome is in index form,
+ * so nn represents zero
+ */
+ if (s[i] != nn)
+ goto decode;
+ }
+
+ /* syndrome is zero, no errors to correct */
+ return 0;
+ }
/* form the syndromes; i.e., evaluate data(x) at roots of
* g(x) */
--
2.17.2
^ permalink raw reply related
* [PATCH v2 4/7] rslib: decode_rs: code cleanup
From: Ferdinand Blomqvist @ 2019-06-20 14:10 UTC (permalink / raw)
To: linux-kernel; +Cc: Thomas Gleixner
In-Reply-To: <20190620141039.9874-1-ferdinand.blomqvist@gmail.com>
Nothing useful was done after the finish label when count is negative so
return directly instead of jumping to finish.
Signed-off-by: Ferdinand Blomqvist <ferdinand.blomqvist@gmail.com>
---
lib/reed_solomon/decode_rs.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/lib/reed_solomon/decode_rs.c b/lib/reed_solomon/decode_rs.c
index 22006eaa41e6..78629bbe6590 100644
--- a/lib/reed_solomon/decode_rs.c
+++ b/lib/reed_solomon/decode_rs.c
@@ -88,8 +88,7 @@
/* if syndrome is zero, data[] is a codeword and there are no
* errors to correct. So return data[] unmodified
*/
- count = 0;
- goto finish;
+ return 0;
}
decode:
@@ -202,8 +201,7 @@
* deg(lambda) unequal to number of roots => uncorrectable
* error detected
*/
- count = -EBADMSG;
- goto finish;
+ return -EBADMSG;
}
/*
* Compute err+eras evaluator poly omega(x) = s(x)*lambda(x) (modulo
@@ -261,7 +259,6 @@
}
}
-finish:
if (eras_pos != NULL) {
for (i = 0; i < count; i++)
eras_pos[i] = loc[i] - pad;
--
2.17.2
^ permalink raw reply related
* [PATCH v2 1/7] rslib: Add tests for the encoder and decoder
From: Ferdinand Blomqvist @ 2019-06-20 14:10 UTC (permalink / raw)
To: linux-kernel; +Cc: Thomas Gleixner
In-Reply-To: <20190620141039.9874-1-ferdinand.blomqvist@gmail.com>
A Reed-Solomon code with minimum distance d can correct any error and
erasure pattern that satisfies 2 * #error + #erasures < d. If the
error correction capacity is exceeded, then correct decoding cannot be
guaranteed. The decoder must, however, return a valid codeword or report
failure.
There are two main tests:
- Check for correct behaviour up to the error correction capacity
- Check for correct behaviour beyond error corrupted capacity
Both tests are simple:
1. Generate random data
2. Encode data with the chosen code
3. Add errors and erasures to data
4. Decode the corrupted word
5. Check for correct behaviour
When testing up to capacity we test for:
- Correct decoding
- Correct return value (i.e. the number of corrected symbols)
- That the returned error positions are correct
There are two kinds of erasures; the erased symbol can be corrupted or
not. When counting the number of corrected symbols, erasures without
symbol corruption should not be counted. Similarly, the returned error
positions should only include positions where a correction is necessary.
We run the up to capacity tests for three different interfaces of
decode_rs:
- Use the correction buffers
- Use the correction buffers with syndromes provided by the caller
- Error correction in place (does not check the error positions)
When testing beyond capacity we test for silent failures. A silent
failure is when the decoder returns success but the returned word is not
a valid codeword.
There are a couple of options for the tests:
- Verbosity.
- Whether to test for correct behaviour beyond capacity. Default is to
test beyond capacity.
- Whether to allow erasures without symbol corruption. Defaults to yes.
Note that the tests take a couple of minutes to complete.
Signed-off-by: Ferdinand Blomqvist <ferdinand.blomqvist@gmail.com>
---
lib/Kconfig.debug | 12 +
lib/reed_solomon/Makefile | 2 +-
lib/reed_solomon/test_rslib.c | 516 ++++++++++++++++++++++++++++++++++
3 files changed, 529 insertions(+), 1 deletion(-)
create mode 100644 lib/reed_solomon/test_rslib.c
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index cbdfae379896..b0d71d9293dc 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1754,6 +1754,18 @@ config RBTREE_TEST
A benchmark measuring the performance of the rbtree library.
Also includes rbtree invariant checks.
+config REED_SOLOMON_TEST
+ tristate "Reed-Solomon library test"
+ depends on DEBUG_KERNEL || m
+ select REED_SOLOMON
+ select REED_SOLOMON_ENC16
+ select REED_SOLOMON_DEC16
+ help
+ This option enables the self-test function of rslib at boot,
+ or at module load time.
+
+ If unsure, say N.
+
config INTERVAL_TREE_TEST
tristate "Interval tree test"
depends on DEBUG_KERNEL
diff --git a/lib/reed_solomon/Makefile b/lib/reed_solomon/Makefile
index ba9d7a3329eb..5d4fa68f26cb 100644
--- a/lib/reed_solomon/Makefile
+++ b/lib/reed_solomon/Makefile
@@ -4,4 +4,4 @@
#
obj-$(CONFIG_REED_SOLOMON) += reed_solomon.o
-
+obj-$(CONFIG_REED_SOLOMON_TEST) += test_rslib.o
diff --git a/lib/reed_solomon/test_rslib.c b/lib/reed_solomon/test_rslib.c
new file mode 100644
index 000000000000..aa40e7ba5824
--- /dev/null
+++ b/lib/reed_solomon/test_rslib.c
@@ -0,0 +1,516 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Tests for Generic Reed Solomon encoder / decoder library
+ *
+ * Written by Ferdinand Blomqvist
+ * Based on previous work by Phil Karn, KA9Q
+ */
+#include <linux/rslib.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/random.h>
+#include <linux/slab.h>
+
+enum verbosity {
+ V_SILENT,
+ V_PROGRESS,
+ V_CSUMMARY
+};
+
+enum method {
+ CORR_BUFFER,
+ CALLER_SYNDROME,
+ IN_PLACE
+};
+
+#define __param(type, name, init, msg) \
+ static type name = init; \
+ module_param(name, type, 0444); \
+ MODULE_PARM_DESC(name, msg)
+
+__param(int, v, V_PROGRESS, "Verbosity level");
+__param(int, ewsc, 1, "Erasures without symbol corruption");
+__param(int, bc, 1, "Test for correct behaviour beyond error correction capacity");
+
+struct etab {
+ int symsize;
+ int genpoly;
+ int fcs;
+ int prim;
+ int nroots;
+ int ntrials;
+};
+
+/* List of codes to test */
+static struct etab Tab[] = {
+ {2, 0x7, 1, 1, 1, 100000 },
+ {3, 0xb, 1, 1, 2, 100000 },
+ {3, 0xb, 1, 1, 3, 100000 },
+ {3, 0xb, 2, 1, 4, 100000 },
+ {4, 0x13, 1, 1, 4, 10000 },
+ {5, 0x25, 1, 1, 6, 1000 },
+ {6, 0x43, 3, 1, 8, 1000 },
+ {7, 0x89, 1, 1, 14, 500 },
+ {8, 0x11d, 1, 1, 30, 100 },
+ {8, 0x187, 112, 11, 32, 100 },
+ {9, 0x211, 1, 1, 33, 80 },
+ {0, 0, 0, 0, 0, 0},
+};
+
+
+struct estat {
+ int dwrong;
+ int irv;
+ int wepos;
+ int nwords;
+};
+
+struct bcstat {
+ int rfail;
+ int rsuccess;
+ int noncw;
+ int nwords;
+};
+
+struct wspace {
+ uint16_t *c; /* sent codeword */
+ uint16_t *r; /* received word */
+ uint16_t *s; /* syndrome */
+ uint16_t *corr; /* correction buffer */
+ int *errlocs;
+ int *derrlocs;
+};
+
+struct pad {
+ int mult;
+ int shift;
+};
+
+static struct pad pad_coef[] = {
+ { 0, 0 },
+ { 1, 2 },
+ { 1, 1 },
+ { 3, 2 },
+ { 1, 0 },
+};
+
+static void free_ws(struct wspace *ws)
+{
+ if (!ws)
+ return;
+
+ kfree(ws->errlocs);
+ kfree(ws->c);
+ kfree(ws);
+}
+
+static struct wspace *alloc_ws(struct rs_codec *rs)
+{
+ int nroots = rs->nroots;
+ struct wspace *ws;
+ int nn = rs->nn;
+
+ ws = kzalloc(sizeof(*ws), GFP_KERNEL);
+ if (!ws)
+ return NULL;
+
+ ws->c = kmalloc_array(2 * (nn + nroots),
+ sizeof(uint16_t), GFP_KERNEL);
+ if (!ws->c)
+ goto err;
+
+ ws->r = ws->c + nn;
+ ws->s = ws->r + nn;
+ ws->corr = ws->s + nroots;
+
+ ws->errlocs = kmalloc_array(nn + nroots, sizeof(int), GFP_KERNEL);
+ if (!ws->errlocs)
+ goto err;
+
+ ws->derrlocs = ws->errlocs + nn;
+ return ws;
+
+err:
+ free_ws(ws);
+ return NULL;
+}
+
+
+/*
+ * Generates a random codeword and stores it in c. Generates random errors and
+ * erasures, and stores the random word with errors in r. Erasure positions are
+ * stored in derrlocs, while errlocs has one of three values in every position:
+ *
+ * 0 if there is no error in this position;
+ * 1 if there is a symbol error in this position;
+ * 2 if there is an erasure without symbol corruption.
+ *
+ * Returns the number of corrupted symbols.
+ */
+static int get_rcw_we(struct rs_control *rs, struct wspace *ws,
+ int len, int errs, int eras)
+{
+ int nroots = rs->codec->nroots;
+ int *derrlocs = ws->derrlocs;
+ int *errlocs = ws->errlocs;
+ int dlen = len - nroots;
+ int nn = rs->codec->nn;
+ uint16_t *c = ws->c;
+ uint16_t *r = ws->r;
+ int errval;
+ int errloc;
+ int i;
+
+ /* Load c with random data and encode */
+ for (i = 0; i < dlen; i++)
+ c[i] = prandom_u32() & nn;
+
+ memset(c + dlen, 0, nroots * sizeof(*c));
+ encode_rs16(rs, c, dlen, c + dlen, 0);
+
+ /* Make copyand add errors and erasures */
+ memcpy(r, c, len * sizeof(*r));
+ memset(errlocs, 0, len * sizeof(*errlocs));
+ memset(derrlocs, 0, nroots * sizeof(*derrlocs));
+
+ /* Generating random errors */
+ for (i = 0; i < errs; i++) {
+ do {
+ /* Error value must be nonzero */
+ errval = prandom_u32() & nn;
+ } while (errval == 0);
+
+ do {
+ /* Must not choose the same location twice */
+ errloc = prandom_u32() % len;
+ } while (errlocs[errloc] != 0);
+
+ errlocs[errloc] = 1;
+ r[errloc] ^= errval;
+ }
+
+ /* Generating random erasures */
+ for (i = 0; i < eras; i++) {
+ do {
+ /* Must not choose the same location twice */
+ errloc = prandom_u32() % len;
+ } while (errlocs[errloc] != 0);
+
+ derrlocs[i] = errloc;
+
+ if (ewsc && (prandom_u32() & 1)) {
+ /* Erasure with the symbol intact */
+ errlocs[errloc] = 2;
+ } else {
+ /* Erasure with corrupted symbol */
+ do {
+ /* Error value must be nonzero */
+ errval = prandom_u32() & nn;
+ } while (errval == 0);
+
+ errlocs[errloc] = 1;
+ r[errloc] ^= errval;
+ errs++;
+ }
+ }
+
+ return errs;
+}
+
+static void fix_err(uint16_t *data, int nerrs, uint16_t *corr, int *errlocs)
+{
+ int i;
+
+ for (i = 0; i < nerrs; i++)
+ data[errlocs[i]] ^= corr[i];
+}
+
+static void compute_syndrome(struct rs_control *rsc, uint16_t *data,
+ int len, uint16_t *syn)
+{
+ struct rs_codec *rs = rsc->codec;
+ uint16_t *alpha_to = rs->alpha_to;
+ uint16_t *index_of = rs->index_of;
+ int nroots = rs->nroots;
+ int prim = rs->prim;
+ int fcr = rs->fcr;
+ int i, j;
+
+ /* Calculating syndrome */
+ for (i = 0; i < nroots; i++) {
+ syn[i] = data[0];
+ for (j = 1; j < len; j++) {
+ if (syn[i] == 0) {
+ syn[i] = data[j];
+ } else {
+ syn[i] = data[j] ^
+ alpha_to[rs_modnn(rs, index_of[syn[i]]
+ + (fcr + i) * prim)];
+ }
+ }
+ }
+
+ /* Convert to index form */
+ for (i = 0; i < nroots; i++)
+ syn[i] = rs->index_of[syn[i]];
+}
+
+/* Test up to error correction capacity */
+static void test_uc(struct rs_control *rs, int len, int errs,
+ int eras, int trials, struct estat *stat,
+ struct wspace *ws, int method)
+{
+ int dlen = len - rs->codec->nroots;
+ int *derrlocs = ws->derrlocs;
+ int *errlocs = ws->errlocs;
+ uint16_t *corr = ws->corr;
+ uint16_t *c = ws->c;
+ uint16_t *r = ws->r;
+ uint16_t *s = ws->s;
+ int derrs, nerrs;
+ int i, j;
+
+ for (j = 0; j < trials; j++) {
+ nerrs = get_rcw_we(rs, ws, len, errs, eras);
+
+ switch (method) {
+ case CORR_BUFFER:
+ derrs = decode_rs16(rs, r, r + dlen, dlen,
+ NULL, eras, derrlocs, 0, corr);
+ fix_err(r, derrs, corr, derrlocs);
+ break;
+ case CALLER_SYNDROME:
+ compute_syndrome(rs, r, len, s);
+ derrs = decode_rs16(rs, NULL, NULL, dlen,
+ s, eras, derrlocs, 0, corr);
+ fix_err(r, derrs, corr, derrlocs);
+ break;
+ case IN_PLACE:
+ derrs = decode_rs16(rs, r, r + dlen, dlen,
+ NULL, eras, derrlocs, 0, NULL);
+ break;
+ }
+
+ if (derrs != nerrs)
+ stat->irv++;
+
+ if (method != IN_PLACE) {
+ for (i = 0; i < derrs; i++) {
+ if (errlocs[derrlocs[i]] != 1)
+ stat->wepos++;
+ }
+ }
+
+ if (memcmp(r, c, len * sizeof(*r)))
+ stat->dwrong++;
+ }
+ stat->nwords += trials;
+}
+
+int ex_rs_helper(struct rs_control *rs, struct wspace *ws,
+ int len, int trials, int method)
+{
+ static const char * const desc[] = {
+ "Testing correction buffer interface...",
+ "Testing with caller provided syndrome...",
+ "Testing in-place interface..."
+ };
+
+ struct estat stat = {0, 0, 0, 0};
+ int nroots = rs->codec->nroots;
+ int errs, eras, retval;
+
+ if (v >= V_PROGRESS)
+ pr_info(" %s\n", desc[method]);
+
+ for (errs = 0; errs <= nroots / 2; errs++)
+ for (eras = 0; eras <= nroots - 2 * errs; eras++)
+ test_uc(rs, len, errs, eras, trials, &stat, ws, method);
+
+ if (v >= V_CSUMMARY) {
+ pr_info(" Decodes wrong: %d / %d\n",
+ stat.dwrong, stat.nwords);
+ pr_info(" Wrong return value: %d / %d\n",
+ stat.irv, stat.nwords);
+ if (method != IN_PLACE)
+ pr_info(" Wrong error position: %d\n", stat.wepos);
+ }
+
+ retval = stat.dwrong + stat.wepos + stat.irv;
+ if (retval && v >= V_PROGRESS)
+ pr_warn(" FAIL: %d decoding failures!\n", retval);
+
+ return retval;
+}
+
+int exercise_rs(struct rs_control *rs, struct wspace *ws,
+ int len, int trials)
+{
+
+ int retval = 0;
+ int i;
+
+ if (v >= V_PROGRESS)
+ pr_info("Testing up to error correction capacity...\n");
+
+ for (i = 0; i <= IN_PLACE; i++)
+ retval |= ex_rs_helper(rs, ws, len, trials, i);
+
+ return retval;
+}
+
+/* Tests for correct behaviour beyond error correction capacity */
+static void test_bc(struct rs_control *rs, int len, int errs,
+ int eras, int trials, struct bcstat *stat,
+ struct wspace *ws)
+{
+ int nroots = rs->codec->nroots;
+ int dlen = len - nroots;
+ int *derrlocs = ws->derrlocs;
+ uint16_t *corr = ws->corr;
+ uint16_t *r = ws->r;
+ int derrs, j;
+
+ for (j = 0; j < trials; j++) {
+ get_rcw_we(rs, ws, len, errs, eras);
+ derrs = decode_rs16(rs, r, r + dlen, dlen,
+ NULL, eras, derrlocs, 0, corr);
+ fix_err(r, derrs, corr, derrlocs);
+
+ if (derrs >= 0) {
+ stat->rsuccess++;
+
+ /*
+ * We check that the returned word is actually a
+ * codeword. The obious way to do this would be to
+ * compute the syndrome, but we don't want to replicate
+ * that code here. However, all the codes are in
+ * systematic form, and therefore we can encode the
+ * returned word, and see whether the parity changes or
+ * not.
+ */
+ memset(corr, 0, nroots * sizeof(*corr));
+ encode_rs16(rs, r, dlen, corr, 0);
+
+ if (memcmp(r + dlen, corr, nroots * sizeof(*corr)))
+ stat->noncw++;
+ } else {
+ stat->rfail++;
+ }
+ }
+ stat->nwords += trials;
+}
+
+int exercise_rs_bc(struct rs_control *rs, struct wspace *ws,
+ int len, int trials)
+{
+ struct bcstat stat = {0, 0, 0, 0};
+ int nroots = rs->codec->nroots;
+ int errs, eras, cutoff;
+
+ if (v >= V_PROGRESS)
+ pr_info("Testing beyond error correction capacity...\n");
+
+ for (errs = 1; errs <= nroots; errs++) {
+ eras = nroots - 2 * errs + 1;
+ if (eras < 0)
+ eras = 0;
+
+ cutoff = nroots <= len - errs ? nroots : len - errs;
+ for (; eras <= cutoff; eras++)
+ test_bc(rs, len, errs, eras, trials, &stat, ws);
+ }
+
+ if (v >= V_CSUMMARY) {
+ pr_info(" decoder gives up: %d / %d\n",
+ stat.rfail, stat.nwords);
+ pr_info(" decoder returns success: %d / %d\n",
+ stat.rsuccess, stat.nwords);
+ pr_info(" not a codeword: %d / %d\n",
+ stat.noncw, stat.rsuccess);
+ }
+
+ if (stat.noncw && v >= V_PROGRESS)
+ pr_warn(" FAIL: %d silent failures!\n", stat.noncw);
+
+ return stat.noncw;
+}
+
+static int run_exercise(struct etab *e)
+{
+ int nn = (1 << e->symsize) - 1;
+ int kk = nn - e->nroots;
+ struct rs_control *rsc;
+ int retval = -ENOMEM;
+ int max_pad = kk - 1;
+ int prev_pad = -1;
+ struct wspace *ws;
+ int i;
+
+ rsc = init_rs(e->symsize, e->genpoly, e->fcs, e->prim, e->nroots);
+ if (!rsc)
+ return retval;
+
+ ws = alloc_ws(rsc->codec);
+ if (!ws)
+ goto err;
+
+ retval = 0;
+ for (i = 0; i < ARRAY_SIZE(pad_coef); i++) {
+ int pad = (pad_coef[i].mult * max_pad) >> pad_coef[i].shift;
+ int len = nn - pad;
+
+ if (pad == prev_pad)
+ continue;
+
+ prev_pad = pad;
+ if (v >= V_PROGRESS) {
+ pr_info("Testing (%d,%d)_%d code...\n",
+ len, kk - pad, nn + 1);
+ }
+
+ retval |= exercise_rs(rsc, ws, len, e->ntrials);
+ if (bc)
+ retval |= exercise_rs_bc(rsc, ws, len, e->ntrials);
+ }
+
+ free_ws(ws);
+
+err:
+ free_rs(rsc);
+ return retval;
+}
+
+static int __init test_rslib_init(void)
+{
+ int fail, i;
+
+ for (i = 0; Tab[i].symsize != 0 ; i++) {
+ int retval;
+
+ retval = run_exercise(Tab + i);
+ if (retval < 0)
+ return -ENOMEM;
+
+ fail |= retval;
+ }
+
+ if (fail)
+ pr_warn("rslib: test failed\n");
+ else
+ pr_info("rslib: test ok\n");
+
+ return -EAGAIN; /* Fail will directly unload the module */
+}
+
+static void __exit test_rslib_exit(void)
+{
+}
+
+module_init(test_rslib_init)
+module_exit(test_rslib_exit)
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Ferdinand Blomqvist");
+MODULE_DESCRIPTION("Reed-Solomon library test");
--
2.17.2
^ permalink raw reply related
* [PATCH v2 0/7] rslib: RS decoder is severely broken
From: Ferdinand Blomqvist @ 2019-06-20 14:10 UTC (permalink / raw)
To: linux-kernel; +Cc: Thomas Gleixner
This is the second revision of this series that fixes the bugs/flaws in
the RS decoder. This addresses all of Thomas' comments/suggestions.
Changes in v2:
- Replace code that requires the FPU (one small thing in test_rslib.c)
- More verbose changelog (patch 2/7)
- Clarifying comment (patch 7/7)
- Fixed some small whitespace issues (patches 1 and 5)
------
The Reed_Solomon library used in the kernel is based on Phil Karn's
fec library. When playing with this library I found a couple of bugs. It
turn out that all of these bugs, and some additional flaws, are present
in rslib.
The Reed-Solomon decoder has several bugs/flaws:
- Decoding of shortened codes is broken. The decoder only works as
expected if there are no erasures.
- The decoder sometimes fails silently, i.e. it announces success but
returns a word that is not a codeword.
- The return value of the decoder is incoherent with respect to how
fixed erasures are counted. If the word to be decoded is a codeword,
then the decoder always returns zero even if some erasures are given.
On the other hand, if the word to be decoded contains errors, then the
number of erasures is always included in the count of corrected
symbols. So the decoder handles erasures without symbol corruption
inconsistently. This inconsistency probably doesn't affect anyone
using the decoder, but it is inconsistent with the documentation.
- The error positions returned in eras_pos include all erasures, but the
corrections are only set in the correction buffer if there actually is
a symbol error. So if there are erasures without symbol corruption,
then the correction buffer will contain errors (unless initialized to
zero before calling the decoder) or some values will be unset (if the
correction buffer is uninitialized).
- Assumes that the syndromes provided by callers are in index form.
This is simply undocumented.
- When correcting data in-place the decoder does not correct errors in
the parity. On the other hand, when returning the errors in correction
buffers, errors in the parity are included.
This series provides a module with tests for rslib and fixes all the
bugs/flaws. I am not sure that the provided self tests are written in
the 'right way'. I just looked at other self tests in lib and
implemented something similar.
The fixes are tested with the self tests. They should probably also be
tested with drivers etc. that use rslib, but it is unclear to me how to
do this.
I looked a bit on two of the drivers that use rslib:
drivers/mtd/nand/raw/cafe_nand.c
drivers/mtd/nand/raw/diskonchip.c
Both of them seem to do some additional error checking after calling
decode_rs16. Maybe this is needed because of the bugs in the decoder?
Ferdinand Blomqvist (7):
rslib: Add tests for the encoder and decoder
rslib: Fix decoding of shortened codes
rslib: decode_rs: Fix length parameter check
rslib: decode_rs: code cleanup
rslib: Fix handling of of caller provided syndrome
rslib: Update documentation
rslib: Fix remaining decoder flaws
lib/Kconfig.debug | 12 +
lib/reed_solomon/Makefile | 2 +-
lib/reed_solomon/decode_rs.c | 115 +++++--
lib/reed_solomon/reed_solomon.c | 12 +-
lib/reed_solomon/test_rslib.c | 516 ++++++++++++++++++++++++++++++++
5 files changed, 622 insertions(+), 35 deletions(-)
create mode 100644 lib/reed_solomon/test_rslib.c
--
2.17.2
^ permalink raw reply
* Re: [Qemu-devel] [PATCH RESEND v5 0/3] fw_cfg: Add edk2_add_host_crypto_policy()
From: Philippe Mathieu-Daudé @ 2019-06-20 13:55 UTC (permalink / raw)
To: qemu-devel, Laszlo Ersek, Igor Mammedov
Cc: Peter Maydell, Andrew Jones, Eduardo Habkost, Michael S. Tsirkin,
qemu-arm, Paolo Bonzini
In-Reply-To: <20190620122132.10075-1-philmd@redhat.com>
Cc'ing Igor (suggested by Laszlo, for his QOM experience on
UserCreatableClass).
On 6/20/19 2:21 PM, Philippe Mathieu-Daudé wrote:
> Hi,
>
> This series add edk2_add_host_crypto_policy() and the Edk2Crypto object.
>
> The Edk2Crypto object is used to hold configuration values specific
> to EDK2.
>
> So far only the 'https' policy is supported.
>
> A usercase example is the 'HTTPS Boof' feature of OVMF [*].
>
> Usage example:
>
> $ qemu-system-x86_64 \
> --object edk2_crypto,id=https,\
> ciphers=/etc/crypto-policies/back-ends/openssl.config,\
> cacerts=/etc/pki/ca-trust/extracted/edk2/cacerts.bin
>
> (On Fedora these files are provided by the ca-certificates and
> crypto-policies packages).
>
> [*]: https://github.com/tianocore/edk2/blob/master/OvmfPkg/README
>
> Since v4:
> - Addressed Laszlo comments (see patch#1 description)
> Since v3:
> - Addressed Markus' comments (do not care about heap)
> Since v2:
> - Split of
> Since v1:
> - Addressed Michael and Laszlo comments.
>
> Please review,
>
> Phil.
>
> $ git backport-diff -u fw_cfg_edk2_crypto_policies-v3
> Key:
> [####] : number of functional differences between upstream/downstream patch
> The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
>
> 001/3:[0164] [FC] 'hw/firmware: Add Edk2Crypto and edk2_add_host_crypto_policy()'
> 002/3:[----] [--] 'hw/i386: Use edk2_add_host_crypto_policy()'
> 003/3:[----] [--] 'hw/arm/virt: Use edk2_add_host_crypto_policy()'
>
> v4: https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg04300.html
> v3: https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg02965.html
> v2: https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg02522.html
> v1: https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg01598.html
>
> Philippe Mathieu-Daudé (3):
> hw/firmware: Add Edk2Crypto and edk2_add_host_crypto_policy()
> hw/i386: Use edk2_add_host_crypto_policy()
> hw/arm/virt: Use edk2_add_host_crypto_policy()
>
> MAINTAINERS | 2 +
> hw/Makefile.objs | 1 +
> hw/arm/virt.c | 7 +
> hw/firmware/Makefile.objs | 1 +
> hw/firmware/uefi_edk2_crypto_policies.c | 209 ++++++++++++++++++++++++
> hw/i386/pc.c | 7 +
> include/hw/firmware/uefi_edk2.h | 30 ++++
> 7 files changed, 257 insertions(+)
> create mode 100644 hw/firmware/Makefile.objs
> create mode 100644 hw/firmware/uefi_edk2_crypto_policies.c
> create mode 100644 include/hw/firmware/uefi_edk2.h
>
^ permalink raw reply
* Re: [PATCH nf-next v2] netfilter: nf_tables: Add SYNPROXY support
From: Pablo Neira Ayuso @ 2019-06-20 14:10 UTC (permalink / raw)
To: Fernando Fernandez Mancera; +Cc: netfilter-devel
In-Reply-To: <20190619180654.1432-1-ffmancera@riseup.net>
On Wed, Jun 19, 2019 at 08:06:54PM +0200, Fernando Fernandez Mancera wrote:
[...]
> diff --git a/net/netfilter/nft_synproxy.c b/net/netfilter/nft_synproxy.c
> new file mode 100644
> index 000000000000..3ef7f1dc50be
> --- /dev/null
> +++ b/net/netfilter/nft_synproxy.c
> @@ -0,0 +1,327 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
Remove empty line above.
> +#include <linux/types.h>
> +
Same here.
> +#include <net/ip.h>
> +#include <net/tcp.h>
> +#include <net/netlink.h>
> +
Same here.
> +#include <net/netfilter/nf_tables.h>
> +#include <net/netfilter/nf_conntrack.h>
> +#include <net/netfilter/nf_conntrack_ecache.h>
I don't think we need this ecache.h header.
> +#include <net/netfilter/nf_conntrack_extend.h>
You can remove _extend.h, already included by _synproxy.h
> +#include <net/netfilter/nf_conntrack_seqadj.h>
Do we need _seqadj.h?
> +#include <net/netfilter/nf_conntrack_synproxy.h>
> +#include <net/netfilter/nf_synproxy.h>
> +
remove empty line.
> +#include <linux/netfilter/nf_tables.h>
> +#include <linux/netfilter/nf_SYNPROXY.h>
> +
> +struct nft_synproxy {
> + struct nf_synproxy_info info;
> +};
> +
> +static const struct nla_policy nft_synproxy_policy[NFTA_SYNPROXY_MAX + 1] = {
> + [NFTA_SYNPROXY_MSS] = { .type = NLA_U16 },
> + [NFTA_SYNPROXY_WSCALE] = { .type = NLA_U8 },
> + [NFTA_SYNPROXY_FLAGS] = { .type = NLA_U32 },
> +};
> +
> +static void nft_synproxy_eval_v4(const struct nft_expr *expr,
> + struct nft_regs *regs,
> + const struct nft_pktinfo *pkt)
> +{
> + struct nft_synproxy *priv = nft_expr_priv(expr);
> + struct nf_synproxy_info info = priv->info;
> + struct synproxy_options opts = {};
> + struct net *net = nft_net(pkt);
> + struct synproxy_net *snet = synproxy_pernet(net);
> + struct sk_buff *skb = pkt->skb;
> + int thoff = pkt->xt.thoff;
> + const struct tcphdr *tcp;
> + struct tcphdr _tcph;
> +
> + if (nf_ip_checksum(skb, nft_hook(pkt), thoff, IPPROTO_TCP)) {
> + regs->verdict.code = NF_DROP;
> + return;
> + }
> +
> + tcp = skb_header_pointer(skb, ip_hdrlen(skb),
> + sizeof(struct tcphdr), &_tcph);
> + if (!tcp) {
> + regs->verdict.code = NF_DROP;
> + return;
> + }
This code above is common to eval_v6, you can place it in _eval(),
and pass the pointer tcph to _eval_v4().
> + if (!synproxy_parse_options(skb, thoff, tcp, &opts)) {
> + regs->verdict.code = NF_DROP;
> + return;
> + }
> +
> + if (tcp->syn) {
> + /* Initial SYN from client */
> + this_cpu_inc(snet->stats->syn_received);
> +
> + if (tcp->ece && tcp->cwr)
> + opts.options |= NF_SYNPROXY_OPT_ECN;
> +
> + opts.options &= priv->info.options;
> + if (opts.options & NF_SYNPROXY_OPT_TIMESTAMP)
> + synproxy_init_timestamp_cookie(&info, &opts);
> + else
> + opts.options &= ~(NF_SYNPROXY_OPT_WSCALE |
> + NF_SYNPROXY_OPT_SACK_PERM |
> + NF_SYNPROXY_OPT_ECN);
> +
> + synproxy_send_client_synack(net, skb, tcp, &opts);
> + consume_skb(skb);
> + regs->verdict.code = NF_STOLEN;
> + return;
> + } else if (tcp->ack) {
> + /* ACK from client */
> + if (synproxy_recv_client_ack(net, skb, tcp, &opts,
> + ntohl(tcp->seq))) {
> + consume_skb(skb);
> + regs->verdict.code = NF_STOLEN;
> + } else {
> + regs->verdict.code = NF_DROP;
> + }
> + return;
> + }
> +
> + regs->verdict.code = NFT_CONTINUE;
> +}
> +
> +#if IS_ENABLED(CONFIG_NF_TABLES_IPV6)
> +static void nft_synproxy_eval_v6(const struct nft_expr *expr,
> + struct nft_regs *regs,
> + const struct nft_pktinfo *pkt)
> +{
> + struct nft_synproxy *priv = nft_expr_priv(expr);
> + struct nf_synproxy_info info = priv->info;
> + struct synproxy_options opts = {};
> + struct net *net = nft_net(pkt);
> + struct synproxy_net *snet = synproxy_pernet(net);
> + struct sk_buff *skb = pkt->skb;
> + int thoff = pkt->xt.thoff;
> + const struct tcphdr *tcp;
> + struct tcphdr _tcph;
> +
> + if (nf_ip_checksum(skb, nft_hook(pkt), thoff, IPPROTO_TCP)) {
> + regs->verdict.code = NF_DROP;
> + return;
> + }
> +
> + tcp = skb_header_pointer(skb, ip_hdrlen(skb),
ip_hdrlen() won't fly for IPv6.
> + sizeof(struct tcphdr), &_tcph);
> + if (!tcp) {
> + regs->verdict.code = NF_DROP;
> + return;
> + }
> +
> + if (!synproxy_parse_options(skb, thoff, tcp, &opts)) {
> + regs->verdict.code = NF_DROP;
> + return;
> + }
> +
> + if (tcp->syn) {
From here...
> + /* Initial SYN from client */
> + this_cpu_inc(snet->stats->syn_received);
> +
> + if (tcp->ece && tcp->cwr)
> + opts.options |= NF_SYNPROXY_OPT_ECN;
> +
> + opts.options &= priv->info.options;
> + if (opts.options & NF_SYNPROXY_OPT_TIMESTAMP)
> + synproxy_init_timestamp_cookie(&info, &opts);
> + else
> + opts.options &= ~(NF_SYNPROXY_OPT_WSCALE |
> + NF_SYNPROXY_OPT_SACK_PERM |
> + NF_SYNPROXY_OPT_ECN);
... to here. Could you wrap this code into a function? eg.
nft_synproxy_tcp_options(...);
that can be shared by v4 and v6.
> +
> + synproxy_send_client_synack_ipv6(net, skb, tcp, &opts);
> + consume_skb(skb);
> + regs->verdict.code = NF_STOLEN;
> + return;
> + } else if (tcp->ack) {
> + /* ACK from client */
> + if (synproxy_recv_client_ack_ipv6(net, skb, tcp, &opts,
> + ntohl(tcp->seq))) {
> + consume_skb(skb);
> + regs->verdict.code = NF_STOLEN;
> + } else {
> + regs->verdict.code = NF_DROP;
> + }
> + return;
> + }
> +
> + regs->verdict.code = NFT_CONTINUE;
> +}
> +#endif /* CONFIG_NF_TABLES_IPV6*/
> +
> +static void nft_synproxy_eval(const struct nft_expr *expr,
> + struct nft_regs *regs,
> + const struct nft_pktinfo *pkt)
> +{
> + struct sk_buff *skb = pkt->skb;
> + const struct tcphdr *tcp;
> + struct tcphdr _tcph;
> +
> + tcp = skb_header_pointer(skb, ip_hdrlen(skb),
> + sizeof(struct tcphdr), &_tcph);
> + if (!tcp) {
> + regs->verdict.code = NFT_BREAK;
> + return;
> + }
Hm. You should check for pkt->tprot to check if this is really TCP
before trying to fetch the tcp header.
> + switch (skb->protocol) {
> + case htons(ETH_P_IP):
> + nft_synproxy_eval_v4(expr, regs, pkt);
> + return;
> +#if IS_ENABLED(CONFIG_NF_TABLES_IPV6)
> + case htons(ETH_P_IPV6):
> + nft_synproxy_eval_v6(expr, regs, pkt);
> + return;
> +#endif
> + }
> + regs->verdict.code = NFT_BREAK;
> +}
> +
> +static int nft_synproxy_init(const struct nft_ctx *ctx,
> + const struct nft_expr *expr,
> + const struct nlattr * const tb[])
> +{
> + struct synproxy_net *snet = synproxy_pernet(ctx->net);
> + struct nft_synproxy *priv = nft_expr_priv(expr);
> + u32 flags;
> + int err;
> +
> + err = nf_ct_netns_get(ctx->net, ctx->family);
> + if (err)
> + goto nf_ct_failure;
You just:
return err;
here, if nf_ct_netns_get() fails.
> +
> + switch (ctx->family) {
> + case NFPROTO_IPV4:
> + err = nf_synproxy_ipv4_init(snet, ctx->net);
> + if (err)
> + goto nf_ct_failure;
> + snet->hook_ref4++;
Why this manual bump of the reference counter? Doesn't
nf_synproxy_ipv4_init() deal with this?
> + break;
> +#if IS_ENABLED(IPV6)
> + case NFPROTO_IPV6:
> + err = nf_synproxy_ipv6_init(snet, ctx->net);
> + if (err)
> + goto nf_ct_failure;
> + snet->hook_ref6++;
> + break;
> + case NFPROTO_INET:
> + case NFPROTO_BRIDGE:
> + err = nf_synproxy_ipv4_init(snet, ctx->net);
> + if (err)
> + goto nf_ct_failure;
> + err = nf_synproxy_ipv6_init(snet, ctx->net);
> + if (err)
> + goto nf_ct_failure;
> + snet->hook_ref4++;
> + snet->hook_ref6++;
> + break;
> +#endif
> + }
> +
> + if (tb[NFTA_SYNPROXY_MSS])
> + priv->info.mss = ntohs(nla_get_be16(tb[NFTA_SYNPROXY_MSS]));
> + if (tb[NFTA_SYNPROXY_WSCALE])
> + priv->info.wscale = nla_get_u8(tb[NFTA_SYNPROXY_WSCALE]);
> + if (tb[NFTA_SYNPROXY_FLAGS]) {
> + flags = ntohl(nla_get_be32(tb[NFTA_SYNPROXY_FLAGS]));
> + if (flags != 0 && (flags & NF_SYNPROXY_OPT_MASK) == 0)
> + return -EINVAL;
This -EINVAL ignores error nf_ct_failure error path. I suggest you
move this code up to the top of nft_synproxy_init(), before calling
nf_ct_netns_get().
> + priv->info.options = flags;
> + }
> + return 0;
> +
> +nf_ct_failure:
> + nf_ct_netns_put(ctx->net, ctx->family);
> + return err;
> +}
> +
> +static void nft_synproxy_destroy(const struct nft_ctx *ctx,
> + const struct nft_expr *expr)
> +{
> + struct synproxy_net *snet = synproxy_pernet(ctx->net);
> +
> + switch (ctx->family) {
> + case NFPROTO_IPV4:
> + nf_synproxy_ipv4_fini(snet, ctx->net);
> + break;
> +#if IS_ENABLED(IPV6)
This should be CONFIG_IPV6, right?
> + case NFPROTO_IPV6:
> + nf_synproxy_ipv6_fini(snet, ctx->net);
> + break;
#endif
for IPV6 here.
> + case NFPROTO_INET:
Missing NFPROTO_BRIDGE here.
> + nf_synproxy_ipv4_fini(snet, ctx->net);
> + nf_synproxy_ipv6_fini(snet, ctx->net);
> + break;
> +#endif
> + }
> + nf_ct_netns_put(ctx->net, ctx->family);
> +}
> +
> +static int nft_synproxy_dump(struct sk_buff *skb, const struct nft_expr *expr)
> +{
> + const struct nft_synproxy *priv = nft_expr_priv(expr);
> +
> + if (nla_put_be16(skb, NFTA_SYNPROXY_MSS, ntohs(priv->info.mss)) ||
> + nla_put_u8(skb, NFTA_SYNPROXY_WSCALE, priv->info.wscale) ||
> + nla_put_be32(skb, NFTA_SYNPROXY_FLAGS, ntohl(priv->info.options)))
> + goto nla_put_failure;
> +
> + return 0;
> +
> +nla_put_failure:
> + return -1;
> +}
> +
> +static int nft_synproxy_validate(const struct nft_ctx *ctx,
> + const struct nft_expr *expr,
> + const struct nft_data **data)
> +{
> + return nft_chain_validate_hooks(ctx->chain, (1 << NF_INET_LOCAL_IN) |
> + (1 << NF_INET_FORWARD));
> +}
> +
> +static struct nft_expr_type nft_synproxy_type;
> +static const struct nft_expr_ops nft_synproxy_ops = {
> + .eval = nft_synproxy_eval,
> + .size = NFT_EXPR_SIZE(sizeof(struct nft_synproxy)),
> + .init = nft_synproxy_init,
> + .destroy = nft_synproxy_destroy,
> + .dump = nft_synproxy_dump,
> + .type = &nft_synproxy_type,
> + .validate = nft_synproxy_validate,
> +};
> +
> +static struct nft_expr_type nft_synproxy_type __read_mostly = {
> + .ops = &nft_synproxy_ops,
> + .name = "synproxy",
> + .owner = THIS_MODULE,
> + .policy = nft_synproxy_policy,
> + .maxattr = NFTA_OSF_MAX,
> +};
> +
> +static int __init nft_synproxy_module_init(void)
> +{
> + return nft_register_expr(&nft_synproxy_type);
> +}
> +
> +static void __exit nft_synproxy_module_exit(void)
> +{
> + return nft_unregister_expr(&nft_synproxy_type);
> +}
> +
> +module_init(nft_synproxy_module_init);
> +module_exit(nft_synproxy_module_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Fernando Fernandez <ffmancera@riseup.net>");
> +MODULE_ALIAS_NFT_EXPR("synproxy");
> --
> 2.20.1
>
^ permalink raw reply
* Re: [RFC] deadlock with flush_work() in UAS
From: Tejun Heo @ 2019-06-20 14:10 UTC (permalink / raw)
To: Alan Stern; +Cc: Oliver Neukum, USB list, Kernel development list
In-Reply-To: <Pine.LNX.4.44L0.1906181156370.1659-100000@iolanthe.rowland.org>
Hello,
On Tue, Jun 18, 2019 at 11:59:39AM -0400, Alan Stern wrote:
> > > Even if you disagree, perhaps we should have a global workqueue with a
> > > permanently set noio flag. It could be shared among multiple drivers
> > > such as uas and the hub driver for purposes like this. (In fact, the
> > > hub driver already has its own dedicated workqueue.)
> >
> > That is a good idea. But does UAS need WQ_MEM_RECLAIM?
>
> These are good questions, and I don't have the answers. Perhaps Tejun
> or someone else on LKML can help.
Any device which may host a filesystem or swap needs to use
WQ_MEM_RECLAIM workqueues on anything which may be used during normal
IOs including e.g. error handling which may be invoked. One
WQ_MEM_RECLAIM workqueue guarantees one level of concurrency for all
its tasks regardless of memory situation, so as long as there's no
interdependence between work items, the workqueue can be shared.
Thanks.
--
tejun
^ permalink raw reply
* Re: [Qemu-devel] [PATCH] deprecate -mem-path fallback to anonymous RAM
From: Dr. David Alan Gilbert @ 2019-06-20 13:34 UTC (permalink / raw)
To: Igor Mammedov; +Cc: libvir-list, qemu-devel, ehabkost
In-Reply-To: <20190620114116.27254-1-imammedo@redhat.com>
* Igor Mammedov (imammedo@redhat.com) wrote:
> Fallback might affect guest or worse whole host performance
> or functionality if backing file were used to share guest RAM
> with another process.
>
> Patch deprecates fallback so that we could remove it in future
> and ensure that QEMU will provide expected behavior and fail if
> it can't use user provided backing file.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Ah yes that's useful because it's a mess for postcopy.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> PS:
> Patch is written on top of
> [PATCH v4 0/3] numa: deprecate '-numa node, mem' and default memory distribution
> to avoid conflicts in qemu-deprecated.texi
>
> numa.c | 4 ++--
> qemu-deprecated.texi | 8 ++++++++
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/numa.c b/numa.c
> index 91a29138a2..53d67b8ad9 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -494,8 +494,8 @@ static void allocate_system_memory_nonnuma(MemoryRegion *mr, Object *owner,
> if (mem_prealloc) {
> exit(1);
> }
> - error_report("falling back to regular RAM allocation.");
> -
> + warn_report("falling back to regular RAM allocation. "
> + "Fallback to RAM allocation is deprecated.");
> /* Legacy behavior: if allocation failed, fall back to
> * regular RAM allocation.
> */
> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
> index 2fe9b72121..2193705644 100644
> --- a/qemu-deprecated.texi
> +++ b/qemu-deprecated.texi
> @@ -112,6 +112,14 @@ QEMU using implicit generic or board specific splitting rule.
> Use @option{memdev} with @var{memory-backend-ram} backend or @option{mem} (if
> it's supported by used machine type) to define mapping explictly instead.
>
> +@subsection -mem-path fallback to RAM (since 4.1)
> +Currently if system memory allocation from file pointed by @option{mem-path}
> +fails, QEMU fallbacks to allocating from anonymous RAM. Which might result
> +in unpredictable behavior since provided backing file wasn't used. In future
> +QEMU will not fallback and fail to start up, so user could fix his/her QEMU/host
> +configuration or explicitly use -m without -mem-path if system memory allocated
> +from anonymous RAM suits usecase.
> +
> @section QEMU Machine Protocol (QMP) commands
>
> @subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0)
> --
> 2.18.1
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply
* Re: [PATCH 3/3] drm/i915/execlists: Force preemption
From: Chris Wilson @ 2019-06-20 14:08 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx
In-Reply-To: <87v9x05pbn.fsf@gaia.fi.intel.com>
Quoting Mika Kuoppala (2019-06-20 15:00:44)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
>
> > If the preempted context takes too long to relinquish control, e.g. it
> > is stuck inside a shader with arbitration disabled, evict that context
> > with an engine reset. This ensures that preemptions are reasonably
> > responsive, providing a tighter QoS for the more important context at
> > the cost of flagging unresponsive contexts more frequently (i.e. instead
> > of using an ~10s hangcheck, we now evict at ~10ms). The challenge of
> > lies in picking a timeout that can be reasonably serviced by HW for
> > typical workloads, balancing the existing clients against the needs for
> > responsiveness.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > ---
> > drivers/gpu/drm/i915/Kconfig.profile | 12 ++++++
> > drivers/gpu/drm/i915/gt/intel_lrc.c | 56 ++++++++++++++++++++++++++--
> > 2 files changed, 65 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/Kconfig.profile b/drivers/gpu/drm/i915/Kconfig.profile
> > index 48df8889a88a..8273d3baafe4 100644
> > --- a/drivers/gpu/drm/i915/Kconfig.profile
> > +++ b/drivers/gpu/drm/i915/Kconfig.profile
> > @@ -25,3 +25,15 @@ config DRM_I915_SPIN_REQUEST
> > May be 0 to disable the initial spin. In practice, we estimate
> > the cost of enabling the interrupt (if currently disabled) to be
> > a few microseconds.
> > +
> > +config DRM_I915_PREEMPT_TIMEOUT
> > + int "Preempt timeout (ms)"
> > + default 10 # milliseconds
> > + help
> > + How long to wait (in milliseconds) for a preemption event to occur
> > + when submitting a new context via execlists. If the current context
> > + does not hit an arbitration point and yield to HW before the timer
> > + expires, the HW will be reset to allow the more important context
> > + to execute.
> > +
> > + May be 0 to disable the timeout.
> > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > index fca79adb4aa3..e8d7deba3e49 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > @@ -889,6 +889,15 @@ enable_timeslice(struct intel_engine_cs *engine)
> > return last && need_timeslice(engine, last);
> > }
> >
> > +static unsigned long preempt_expires(void)
> > +{
> > + unsigned long timeout =
>
> could be const
>
> > + msecs_to_jiffies_timeout(CONFIG_DRM_I915_PREEMPT_TIMEOUT);
> > +
> > + barrier();
>
> This needs a comment. I fail to connect the dots as jiffies
> is volatile by nature.
It's just crossing the 't' and dotting the 'i'. What I was thinking was
we don't want the compiler to load jiffies then compute the timeout. So
barrier() there says that timeout is always computed first. Now since it
is likely to be a function call (but I'm trying to find a way to let it
precompute the constant), it will always be precomputed, but who trusts
a compiler.
> > + return jiffies + timeout;
> > +}
> > +
> > static void execlists_dequeue(struct intel_engine_cs *engine)
> > {
> > struct intel_engine_execlists * const execlists = &engine->execlists;
> > @@ -1220,6 +1229,9 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
> > *port = execlists_schedule_in(last, port - execlists->pending);
> > memset(port + 1, 0, (last_port - port) * sizeof(*port));
> > execlists_submit_ports(engine);
> > +
> > + if (CONFIG_DRM_I915_PREEMPT_TIMEOUT)
> > + mod_timer(&execlists->timer, preempt_expires());
> > }
> > }
> >
> > @@ -1376,13 +1388,48 @@ static void process_csb(struct intel_engine_cs *engine)
> > invalidate_csb_entries(&buf[0], &buf[num_entries - 1]);
> > }
> >
> > -static void __execlists_submission_tasklet(struct intel_engine_cs *const engine)
> > +static bool __execlists_submission_tasklet(struct intel_engine_cs *const engine)
> > {
> > lockdep_assert_held(&engine->active.lock);
> >
> > process_csb(engine);
> > - if (!engine->execlists.pending[0])
> > + if (!engine->execlists.pending[0]) {
> > execlists_dequeue(engine);
> > + return true;
> > + }
> > +
> > + return false;
> > +}
> > +
> > +static void preempt_reset(struct intel_engine_cs *engine)
> > +{
> > + const unsigned int bit = I915_RESET_ENGINE + engine->id;
> > + unsigned long *lock = &engine->i915->gpu_error.flags;
> > +
> > + if (test_and_set_bit(bit, lock))
> > + return;
> > +
> > + tasklet_disable_nosync(&engine->execlists.tasklet);
> > + spin_unlock(&engine->active.lock);
> > +
>
> Why do we need to drop the lock?
We take it again inside the reset, and I am far too lazy to lift it to
the caller :) Disabling the tasklet will prevent other threads from
submitting as we drop the lock.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
* Re: [PATCH 2/2] sha1-file: use OBJECT_INFO_NO_FETCH_IF_MISSING
From: Christian Couder @ 2019-06-20 14:08 UTC (permalink / raw)
To: Derrick Stolee
Cc: Jeff King, git, Junio C Hamano, Johannes Schindelin,
Christian Couder
In-Reply-To: <d88ac28b-638b-89c0-2bfb-634a0bb6ca4c@gmail.com>
On Thu, Jun 20, 2019 at 2:39 PM Derrick Stolee <stolee@gmail.com> wrote:
>
> On 6/20/2019 4:50 AM, Jeff King wrote:
> > On Thu, Jun 20, 2019 at 10:30:26AM +0200, Christian Couder wrote:
> >
> >> Currently the OBJECT_INFO_FOR_PREFETCH flag is used to check
> >> if we should fetch objects from promisor remotes when we
> >> haven't found them elsewhere.
> >>
> >> Now that OBJECT_INFO_NO_FETCH_IF_MISSING exists, let's use
> >> it instead to be more correct in case this new flag is ever
> >> used without OBJECT_INFO_QUICK.
> >
> > I said earlier that this one would need to be tweaked for the new
> > upstream name. But actually, I think it is not necessary after Stolee's
> > patch.
>
> Yes, I believe that 31f5256c82 does an equivalent thing to the
> combination of these patches.
Yeah, I agree. Thanks Stolee for having already fixed that, and sorry
for bothering everyone with this.
Christian.
^ permalink raw reply
* [Buildroot] [git commit] package/busybox: run mdev in daemon mode
From: Thomas Petazzoni @ 2019-06-20 14:08 UTC (permalink / raw)
To: buildroot
commit: https://git.buildroot.net/buildroot/commit/?id=733ea6bb4bec6e9ff9ffd8f7444527f91c72033e
branch: https://git.buildroot.net/buildroot/commit/?id=refs/heads/master
- Enable the mdev daemon mode in Busybox default config
- Update the S10mdev init script to use the daemon mode
Signed-off-by: Titouan Christophe <titouan.christophe@railnova.eu>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
package/busybox/S10mdev | 27 +++++++++++++++++++++------
package/busybox/busybox.config | 1 +
2 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/package/busybox/S10mdev b/package/busybox/S10mdev
index efef4290ba..3c6f47c18c 100644
--- a/package/busybox/S10mdev
+++ b/package/busybox/S10mdev
@@ -1,13 +1,17 @@
#!/bin/sh
#
-# Start mdev....
+# Run the mdev daemon
#
+DAEMON="mdev"
+PIDFILE="/var/run/$DAEMON.pid"
+
start() {
- echo "Starting mdev..."
- echo /sbin/mdev >/proc/sys/kernel/hotplug
- /sbin/mdev -s
+ echo -n "Starting $DAEMON... "
+ start-stop-daemon -S -b -m -p $PIDFILE -x /sbin/mdev -- -df
+ [ $? -eq 0 ] && echo "OK" || echo "ERROR"
+
# coldplug modules
find /sys/ -name modalias -print0 | \
xargs -0 sort -u | \
@@ -15,12 +19,23 @@ start() {
xargs -0 modprobe -abq
}
+stop() {
+ echo -n "Stopping $DAEMON... "
+ start-stop-daemon -K -p $PIDFILE
+ [ $? -eq 0 ] && echo "OK" || echo "ERROR"
+}
+
+restart() {
+ stop
+ start
+}
+
case "$1" in
- start)
+ start|stop|restart)
"$1"
;;
*)
- echo "Usage: $0 start"
+ echo "Usage: $0 {start|stop|restart}"
exit 1
esac
diff --git a/package/busybox/busybox.config b/package/busybox/busybox.config
index 1d9560d655..c28718a725 100644
--- a/package/busybox/busybox.config
+++ b/package/busybox/busybox.config
@@ -628,6 +628,7 @@ CONFIG_FEATURE_MDEV_RENAME=y
# CONFIG_FEATURE_MDEV_RENAME_REGEXP is not set
CONFIG_FEATURE_MDEV_EXEC=y
# CONFIG_FEATURE_MDEV_LOAD_FIRMWARE is not set
+CONFIG_FEATURE_MDEV_DAEMON=y
CONFIG_MESG=y
CONFIG_FEATURE_MESG_ENABLE_ONLY_GROUP=y
CONFIG_MKE2FS=y
^ permalink raw reply related
* [Buildroot] [git commit] package/busybox: convert S10mdev to the canonical init script format
From: Thomas Petazzoni @ 2019-06-20 14:08 UTC (permalink / raw)
To: buildroot
commit: https://git.buildroot.net/buildroot/commit/?id=591adcf6367220d1922069532d1f8ca76e6af916
branch: https://git.buildroot.net/buildroot/commit/?id=refs/heads/master
Signed-off-by: Titouan Christophe <titouan.christophe@railnova.eu>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
package/busybox/S10mdev | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/package/busybox/S10mdev b/package/busybox/S10mdev
index 7075b77016..efef4290ba 100644
--- a/package/busybox/S10mdev
+++ b/package/busybox/S10mdev
@@ -3,21 +3,24 @@
# Start mdev....
#
-case "$1" in
- start)
+
+start() {
echo "Starting mdev..."
echo /sbin/mdev >/proc/sys/kernel/hotplug
/sbin/mdev -s
# coldplug modules
- find /sys/ -name modalias -print0 | xargs -0 sort -u | tr '\n' '\0' | \
- xargs -0 modprobe -abq
- ;;
- stop)
- ;;
- restart|reload)
+ find /sys/ -name modalias -print0 | \
+ xargs -0 sort -u | \
+ tr '\n' '\0' | \
+ xargs -0 modprobe -abq
+}
+
+case "$1" in
+ start)
+ "$1"
;;
*)
- echo "Usage: $0 {start|stop|restart}"
+ echo "Usage: $0 start"
exit 1
esac
^ permalink raw reply related
* Re: crypto: crypto4xx - properly set IV after de- and encrypt breaks kernel 4.4
From: Greg KH @ 2019-06-20 14:07 UTC (permalink / raw)
To: Christian Lamparter
Cc: Hauke Mehrtens, herbert, davem, stable, linux-crypto, Amit Pundir
In-Reply-To: <4226536.PGTo7a8ESG@debian64>
On Thu, Jun 20, 2019 at 02:07:27PM +0200, Christian Lamparter wrote:
> On Thursday, June 20, 2019 11:36:50 AM CEST Hauke Mehrtens wrote:
> > Hi,
> >
> > The patch "crypto: crypto4xx - properly set IV after de- and encrypt"
> > causes a compile error on kernel 4.4.
>
> 3.18 as well.
> >
> > When I revert this commit it compiles for me again:
> > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=e9a60ab1609a7d975922adad1bf9c46ac6954584
> >
> > I do not have hardware to test if it is really working.
>
> I have a few APM821XX. But please note drivers without
>
> commit b66c685a482117d4e9ee987d252ca673689a5302
> Author: Christian Lamparter <chunkeey@gmail.com>
> Date: Fri Dec 22 21:18:36 2017 +0100
>
> crypto: crypto4xx - support Revision B parts
>
> don't work on those and I do have my doubts that 460EX
> series (and older) would either. I also don't believe that
> the inital driver as it was submitted would have worked.
> >From what I've seen in their SDK, they patched the testmgr
> at the time to either disable tests or provided their own...
> so, might as well revert these patches for 4.4 and 3.18.
Now reverted from both, thanks.
greg k-h
^ permalink raw reply
* [PATCH v2 23/23] drm/i915: WARN about invalid lane reversal in TBT-alt/DP-alt modes
From: Imre Deak @ 2019-06-20 14:06 UTC (permalink / raw)
To: intel-gfx
In-Reply-To: <20190620140600.11357-1-imre.deak@intel.com>
Lane reversal happens only in the FIA module for TBT-alt/DP-alt mode, so
WARN if lane reversal is attempted at a different level. See the
BSpec DDI_BUF_CTL register description.
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/display/intel_ddi.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 97611b3bd302..ea7122209ddd 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3604,6 +3604,8 @@ static void intel_ddi_set_fia_lane_count(struct intel_encoder *encoder,
u32 val = I915_READ(PORT_TX_DFLEXDPMLE1);
bool lane_reversal = dig_port->saved_port_bits & DDI_BUF_PORT_REVERSAL;
+ WARN_ON(lane_reversal && dig_port->tc_mode != TC_PORT_LEGACY);
+
val &= ~DFLEXDPMLE1_DPMLETC_MASK(tc_port);
switch (pipe_config->lane_count) {
case 1:
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related
* [PATCH v2 22/23] drm/i915: Remove unneeded disconnect in TypeC legacy port mode
From: Imre Deak @ 2019-06-20 14:05 UTC (permalink / raw)
To: intel-gfx
In-Reply-To: <20190620140600.11357-1-imre.deak@intel.com>
Disconnecting the TypeC PHY when the port is in legacy mode is not
necessary:
- BSpec doesn't specify a disconnect sequence for legacy mode.
- The use of the PHY is dedicated for the display in legacy mode.
- We keep the PHY always connected during runtime as well in legacy
mode.
We disconnect the PHY when needed during a disabling modeset for the
port, so we can also remove the disconnect call from the destroy hook.
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/display/intel_ddi.c | 21 +--------------------
drivers/gpu/drm/i915/display/intel_tc.c | 4 +++-
drivers/gpu/drm/i915/display/intel_tc.h | 2 --
3 files changed, 4 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index ddbc4944b4e0..97611b3bd302 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3946,31 +3946,12 @@ static int intel_ddi_compute_config(struct intel_encoder *encoder,
return 0;
}
-static void intel_ddi_encoder_suspend(struct intel_encoder *encoder)
-{
- struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
-
- intel_dp_encoder_suspend(encoder);
-
- /*
- * TODO: disconnect also from USB DP alternate mode once we have a
- * way to handle the modeset restore in that mode during resume
- * even if the sink has disappeared while being suspended.
- */
- if (dig_port->tc_legacy_port)
- icl_tc_phy_disconnect(dig_port);
-}
-
static void intel_ddi_encoder_destroy(struct drm_encoder *encoder)
{
struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
- struct drm_i915_private *i915 = to_i915(encoder->dev);
intel_dp_encoder_flush_work(encoder);
- if (intel_port_is_tc(i915, dig_port->base.port))
- icl_tc_phy_disconnect(dig_port);
-
drm_encoder_cleanup(encoder);
kfree(dig_port);
}
@@ -4262,7 +4243,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
intel_encoder->update_pipe = intel_ddi_update_pipe;
intel_encoder->get_hw_state = intel_ddi_get_hw_state;
intel_encoder->get_config = intel_ddi_get_config;
- intel_encoder->suspend = intel_ddi_encoder_suspend;
+ intel_encoder->suspend = intel_dp_encoder_suspend;
intel_encoder->get_power_domains = intel_ddi_get_power_domains;
intel_encoder->type = INTEL_OUTPUT_DDI;
intel_encoder->power_domain = intel_port_to_power_domain(port);
diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
index 6c43becf97f7..5e40b7eef7dd 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.c
+++ b/drivers/gpu/drm/i915/display/intel_tc.c
@@ -227,10 +227,12 @@ static void icl_tc_phy_connect(struct intel_digital_port *dig_port,
* See the comment at the connect function. This implements the Disconnect
* Flow.
*/
-void icl_tc_phy_disconnect(struct intel_digital_port *dig_port)
+static void icl_tc_phy_disconnect(struct intel_digital_port *dig_port)
{
switch (dig_port->tc_mode) {
case TC_PORT_LEGACY:
+ /* Nothing to do, we never disconnect from legacy mode */
+ break;
case TC_PORT_DP_ALT:
icl_tc_phy_set_safe_mode(dig_port, true);
dig_port->tc_mode = TC_PORT_TBT_ALT;
diff --git a/drivers/gpu/drm/i915/display/intel_tc.h b/drivers/gpu/drm/i915/display/intel_tc.h
index 8adc107cdbcb..0d8411d4a91d 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.h
+++ b/drivers/gpu/drm/i915/display/intel_tc.h
@@ -10,8 +10,6 @@
#include <linux/mutex.h>
#include "intel_drv.h"
-void icl_tc_phy_disconnect(struct intel_digital_port *dig_port);
-
bool intel_tc_port_connected(struct intel_digital_port *dig_port);
u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port);
int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port);
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related
* [PATCH v2 21/23] drm/i915: Add state verification for the TypeC port mode
From: Imre Deak @ 2019-06-20 14:05 UTC (permalink / raw)
To: intel-gfx
In-Reply-To: <20190620140600.11357-1-imre.deak@intel.com>
Add state verification for the TypeC port mode wrt. the port's AUX power
well enabling/disabling. Also check the correctness of changing the port
mode:
- When enabling/disabling the AUX power well for a TypeC port we must hold
already the TypeC port lock.
- When changing the TypeC port mode the port's AUX power domain must be
disabled.
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
.../drm/i915/display/intel_display_power.c | 99 +++++++++++++++++--
drivers/gpu/drm/i915/display/intel_tc.c | 2 +
drivers/gpu/drm/i915/display/intel_tc.h | 10 +-
3 files changed, 102 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index fd13cd68deae..f4613e10c3b3 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -17,6 +17,7 @@
#include "intel_drv.h"
#include "intel_hotplug.h"
#include "intel_sideband.h"
+#include "intel_tc.h"
bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv,
enum i915_power_well_id power_well_id);
@@ -447,26 +448,110 @@ icl_combo_phy_aux_power_well_disable(struct drm_i915_private *dev_priv,
#define ICL_TBT_AUX_PW_TO_CH(pw_idx) \
((pw_idx) - ICL_PW_CTL_IDX_AUX_TBT1 + AUX_CH_C)
+static enum aux_ch icl_tc_phy_aux_ch(struct drm_i915_private *dev_priv,
+ struct i915_power_well *power_well)
+{
+ int pw_idx = power_well->desc->hsw.idx;
+
+ return power_well->desc->hsw.is_tc_tbt ? ICL_TBT_AUX_PW_TO_CH(pw_idx) :
+ ICL_AUX_PW_TO_CH(pw_idx);
+}
+
+#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
+
+static u64 async_put_domains_mask(struct i915_power_domains *power_domains);
+
+static int power_well_async_ref_count(struct drm_i915_private *dev_priv,
+ struct i915_power_well *power_well)
+{
+ enum intel_display_power_domain domain;
+ u64 async_domains = async_put_domains_mask(&dev_priv->power_domains);
+ int refs = 0;
+
+ for_each_power_domain(domain, power_well->desc->domains)
+ refs += !!(async_domains & BIT_ULL(domain));
+
+ WARN_ON(refs > power_well->count);
+
+ return refs;
+}
+
+static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv,
+ struct i915_power_well *power_well)
+{
+ enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well);
+ struct intel_digital_port *dig_port = NULL;
+ struct intel_encoder *encoder;
+
+ /* Bypass the check if all references are released asynchronously */
+ if (power_well_async_ref_count(dev_priv, power_well) ==
+ power_well->count)
+ return;
+
+ aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well);
+
+ for_each_intel_encoder(&dev_priv->drm, encoder) {
+ if (!intel_port_is_tc(dev_priv, encoder->port))
+ continue;
+
+ /* We'll check the MST primary port */
+ if (encoder->type == INTEL_OUTPUT_DP_MST)
+ continue;
+
+ dig_port = enc_to_dig_port(&encoder->base);
+ if (WARN_ON(!dig_port))
+ continue;
+
+ if (dig_port->aux_ch != aux_ch) {
+ dig_port = NULL;
+ continue;
+ }
+
+ break;
+ }
+
+ if (WARN_ON(!dig_port))
+ return;
+
+ WARN_ON(!intel_tc_port_ref_held(dig_port));
+}
+
+#else
+
+static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv,
+ struct i915_power_well *power_well)
+{
+}
+
+#endif
+
static void
icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv,
struct i915_power_well *power_well)
{
- int pw_idx = power_well->desc->hsw.idx;
- bool is_tbt = power_well->desc->hsw.is_tc_tbt;
- enum aux_ch aux_ch;
+ enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well);
u32 val;
- aux_ch = is_tbt ? ICL_TBT_AUX_PW_TO_CH(pw_idx) :
- ICL_AUX_PW_TO_CH(pw_idx);
+ icl_tc_port_assert_ref_held(dev_priv, power_well);
+
val = I915_READ(DP_AUX_CH_CTL(aux_ch));
val &= ~DP_AUX_CH_CTL_TBT_IO;
- if (is_tbt)
+ if (power_well->desc->hsw.is_tc_tbt)
val |= DP_AUX_CH_CTL_TBT_IO;
I915_WRITE(DP_AUX_CH_CTL(aux_ch), val);
hsw_power_well_enable(dev_priv, power_well);
}
+static void
+icl_tc_phy_aux_power_well_disable(struct drm_i915_private *dev_priv,
+ struct i915_power_well *power_well)
+{
+ icl_tc_port_assert_ref_held(dev_priv, power_well);
+
+ hsw_power_well_disable(dev_priv, power_well);
+}
+
/*
* We should only use the power well if we explicitly asked the hardware to
* enable it, so check if it's enabled and also check if we've requested it to
@@ -3119,7 +3204,7 @@ static const struct i915_power_well_ops icl_combo_phy_aux_power_well_ops = {
static const struct i915_power_well_ops icl_tc_phy_aux_power_well_ops = {
.sync_hw = hsw_power_well_sync_hw,
.enable = icl_tc_phy_aux_power_well_enable,
- .disable = hsw_power_well_disable,
+ .disable = icl_tc_phy_aux_power_well_disable,
.is_enabled = hsw_power_well_enabled,
};
diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
index c8904911e841..6c43becf97f7 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.c
+++ b/drivers/gpu/drm/i915/display/intel_tc.c
@@ -303,6 +303,8 @@ static void intel_tc_port_reset_mode(struct intel_digital_port *dig_port,
enum tc_port_mode old_tc_mode = dig_port->tc_mode;
intel_display_power_flush_work(dev_priv);
+ WARN_ON(intel_display_power_is_enabled(dev_priv,
+ intel_aux_power_domain(dig_port)));
icl_tc_phy_disconnect(dig_port);
icl_tc_phy_connect(dig_port, required_lanes);
diff --git a/drivers/gpu/drm/i915/display/intel_tc.h b/drivers/gpu/drm/i915/display/intel_tc.h
index 31af7be96070..8adc107cdbcb 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.h
+++ b/drivers/gpu/drm/i915/display/intel_tc.h
@@ -7,8 +7,8 @@
#define __INTEL_TC_H__
#include <linux/types.h>
-
-struct intel_digital_port;
+#include <linux/mutex.h>
+#include "intel_drv.h"
void icl_tc_phy_disconnect(struct intel_digital_port *dig_port);
@@ -23,6 +23,12 @@ void intel_tc_port_get_link(struct intel_digital_port *dig_port,
int required_lanes);
void intel_tc_port_put_link(struct intel_digital_port *dig_port);
+static inline int intel_tc_port_ref_held(struct intel_digital_port *dig_port)
+{
+ return mutex_is_locked(&dig_port->tc_lock) ||
+ dig_port->tc_link_refcount;
+}
+
void intel_tc_port_init(struct intel_digital_port *dig_port, bool is_legacy);
#endif /* __INTEL_TC_H__ */
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related
* [PATCH v2 20/23] drm/i915: Keep the TypeC port mode fixed when the port is active
From: Imre Deak @ 2019-06-20 14:05 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter
In-Reply-To: <20190620140600.11357-1-imre.deak@intel.com>
The TypeC port mode needs to stay fixed whenever the port is active. Do
that by introducing a tc_link_refcount to account for active ports,
avoiding changing the port mode if a reference is held.
During the modeset commit phase we also have to reset the port mode and
update the active PLL reflecting the new port mode. We can do this only
once the port and its old PLL has been already disabled. Add the new
encoder update_prepare/complete hooks that are called around the whole
enabling sequence. The TypeC specific hooks of these will reset the port
mode, update the active PLL if the port will be active and ensure that
the port mode will stay fixed for the duration of the whole enabling
sequence by holding a tc_link_refcount.
During the port enabling, the pre_pll_enable/post_pll_disable hooks will
take/release a tc_link_refcount to ensure the port mode stays fixed
while the port is active.
Changing the port mode should also be avoided during connector detection
and AUX transfers if the port is active, we'll do that by checking the
port's tc_link_refcount.
When resetting the port mode we also have to take into account the
maximum lanes provided by the FIA. It's guaranteed to be 4 in TBT-alt
and legacy modes, but there may be less lanes available in DP-alt mode,
in which case we have to fall back to TBT-alt mode.
While at it also update icl_tc_phy_connect()'s code comment, reflecting
the current way of switching the port mode.
v2:
- Add the update_prepare/complete hooks to the encoder instead of the
connector. (Ville)
- Simplify intel_connector_needs_modeset() by removing redundant if.
(Ville)
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
drivers/gpu/drm/i915/display/intel_ddi.c | 41 +++++++-
drivers/gpu/drm/i915/display/intel_display.c | 96 ++++++++++++++++++-
drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 28 +++++-
drivers/gpu/drm/i915/display/intel_dpll_mgr.h | 3 +
drivers/gpu/drm/i915/display/intel_tc.c | 94 +++++++++++++-----
drivers/gpu/drm/i915/display/intel_tc.h | 3 +
drivers/gpu/drm/i915/intel_drv.h | 8 ++
7 files changed, 243 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 0c5bfbd66b19..ddbc4944b4e0 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3623,6 +3623,30 @@ static void intel_ddi_set_fia_lane_count(struct intel_encoder *encoder,
I915_WRITE(PORT_TX_DFLEXDPMLE1, val);
}
+void
+intel_ddi_update_prepare(struct intel_atomic_state *state,
+ struct intel_encoder *encoder,
+ struct intel_crtc *crtc)
+{
+ struct intel_crtc_state *crtc_state =
+ crtc ? intel_atomic_get_new_crtc_state(state, crtc) : NULL;
+ int required_lanes = crtc_state ? crtc_state->lane_count : 1;
+
+ WARN_ON(crtc && crtc->active);
+
+ intel_tc_port_get_link(enc_to_dig_port(&encoder->base), required_lanes);
+ if (crtc_state && crtc_state->base.active)
+ intel_update_active_dpll(state, crtc, encoder);
+}
+
+void
+intel_ddi_update_complete(struct intel_atomic_state *state,
+ struct intel_encoder *encoder,
+ struct intel_crtc *crtc)
+{
+ intel_tc_port_put_link(enc_to_dig_port(&encoder->base));
+}
+
static void
intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
const struct intel_crtc_state *crtc_state,
@@ -3630,10 +3654,13 @@ intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
{
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
+ bool is_tc_port = intel_port_is_tc(dev_priv, encoder->port);
enum port port = encoder->port;
- if (intel_crtc_has_dp_encoder(crtc_state) ||
- intel_port_is_tc(dev_priv, encoder->port))
+ if (is_tc_port)
+ intel_tc_port_get_link(dig_port, crtc_state->lane_count);
+
+ if (intel_crtc_has_dp_encoder(crtc_state) || is_tc_port)
intel_display_power_get(dev_priv,
intel_ddi_main_link_aux_domain(dig_port));
@@ -3658,11 +3685,14 @@ intel_ddi_post_pll_disable(struct intel_encoder *encoder,
{
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
+ bool is_tc_port = intel_port_is_tc(dev_priv, encoder->port);
- if (intel_crtc_has_dp_encoder(crtc_state) ||
- intel_port_is_tc(dev_priv, encoder->port))
+ if (intel_crtc_has_dp_encoder(crtc_state) || is_tc_port)
intel_display_power_put_unchecked(dev_priv,
intel_ddi_main_link_aux_domain(dig_port));
+
+ if (is_tc_port)
+ intel_tc_port_put_link(dig_port);
}
static void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp)
@@ -4256,6 +4286,9 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
!port_info->supports_tbt;
intel_tc_port_init(intel_dig_port, is_legacy);
+
+ intel_encoder->update_prepare = intel_ddi_update_prepare;
+ intel_encoder->update_complete = intel_ddi_update_complete;
}
switch (port) {
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 93e3f568d7db..679307a84062 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -6037,6 +6037,94 @@ static void intel_crtc_disable_planes(struct intel_atomic_state *state,
intel_frontbuffer_flip(dev_priv, fb_bits);
}
+/*
+ * intel_connector_primary_encoder - get the primary encoder for a connector
+ * @connector: connector for which to return the encoder
+ *
+ * Returns the primary encoder for a connector. There is a 1:1 mapping from
+ * all connectors to their encoder, except for DP-MST connectors which have
+ * both a virtual and a primary encoder. These DP-MST primary encoders can be
+ * pointed to by as many DP-MST connectors as there are pipes.
+ */
+static struct intel_encoder *
+intel_connector_primary_encoder(struct intel_connector *connector)
+{
+ struct intel_encoder *encoder;
+
+ if (connector->mst_port)
+ return &dp_to_dig_port(connector->mst_port)->base;
+
+ encoder = intel_attached_encoder(&connector->base);
+ WARN_ON(!encoder);
+
+ return encoder;
+}
+
+static bool
+intel_connector_needs_modeset(struct intel_atomic_state *state,
+ const struct drm_connector_state *old_conn_state,
+ const struct drm_connector_state *new_conn_state)
+{
+ return new_conn_state->crtc != old_conn_state->crtc ||
+ (new_conn_state->crtc &&
+ needs_modeset(drm_atomic_get_new_crtc_state(&state->base,
+ new_conn_state->crtc)));
+}
+
+static void intel_encoders_update_prepare(struct intel_atomic_state *state)
+{
+ struct drm_connector_state *old_conn_state;
+ struct drm_connector_state *new_conn_state;
+ struct drm_connector *conn;
+ int i;
+
+ for_each_oldnew_connector_in_state(&state->base, conn,
+ old_conn_state, new_conn_state, i) {
+ struct intel_encoder *encoder;
+ struct intel_crtc *crtc;
+
+ if (!intel_connector_needs_modeset(state,
+ old_conn_state,
+ new_conn_state))
+ continue;
+
+ encoder = intel_connector_primary_encoder(to_intel_connector(conn));
+ if (!encoder->update_prepare)
+ continue;
+
+ crtc = new_conn_state->crtc ?
+ to_intel_crtc(new_conn_state->crtc) : NULL;
+ encoder->update_prepare(state, encoder, crtc);
+ }
+}
+
+static void intel_encoders_update_complete(struct intel_atomic_state *state)
+{
+ struct drm_connector_state *old_conn_state;
+ struct drm_connector_state *new_conn_state;
+ struct drm_connector *conn;
+ int i;
+
+ for_each_oldnew_connector_in_state(&state->base, conn,
+ old_conn_state, new_conn_state, i) {
+ struct intel_encoder *encoder;
+ struct intel_crtc *crtc;
+
+ if (!intel_connector_needs_modeset(state,
+ old_conn_state,
+ new_conn_state))
+ continue;
+
+ encoder = intel_connector_primary_encoder(to_intel_connector(conn));
+ if (!encoder->update_complete)
+ continue;
+
+ crtc = new_conn_state->crtc ?
+ to_intel_crtc(new_conn_state->crtc) : NULL;
+ encoder->update_complete(state, encoder, crtc);
+ }
+}
+
static void intel_encoders_pre_pll_enable(struct drm_crtc *crtc,
struct intel_crtc_state *crtc_state,
struct drm_atomic_state *old_state)
@@ -13898,14 +13986,20 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
}
}
+ if (intel_state->modeset)
+ intel_encoders_update_prepare(intel_state);
+
/* Now enable the clocks, plane, pipe, and connectors that we set up. */
dev_priv->display.update_crtcs(state);
- if (intel_state->modeset)
+ if (intel_state->modeset) {
+ intel_encoders_update_complete(intel_state);
+
intel_set_cdclk_post_plane_update(dev_priv,
&intel_state->cdclk.actual,
&dev_priv->cdclk.actual,
intel_state->cdclk.pipe);
+ }
/* FIXME: We should call drm_atomic_helper_commit_hw_done() here
* already, but still need the state for the delayed optimization. To
diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index a996a3fad48c..cbb9aa458f19 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -1939,7 +1939,9 @@ struct intel_dpll_mgr {
struct intel_encoder *encoder);
void (*put_dplls)(struct intel_atomic_state *state,
struct intel_crtc *crtc);
-
+ void (*update_active_dpll)(struct intel_atomic_state *state,
+ struct intel_crtc *crtc,
+ struct intel_encoder *encoder);
void (*dump_hw_state)(struct drm_i915_private *dev_priv,
const struct intel_dpll_hw_state *hw_state);
};
@@ -3400,6 +3402,7 @@ static const struct intel_dpll_mgr icl_pll_mgr = {
.dpll_info = icl_plls,
.get_dplls = icl_get_dplls,
.put_dplls = icl_put_dplls,
+ .update_active_dpll = icl_update_active_dpll,
.dump_hw_state = icl_dump_hw_state,
};
@@ -3524,6 +3527,29 @@ void intel_release_shared_dplls(struct intel_atomic_state *state,
dpll_mgr->put_dplls(state, crtc);
}
+/**
+ * intel_update_active_dpll - update the active DPLL for a CRTC/encoder
+ * @state: atomic state
+ * @crtc: the CRTC for which to update the active DPLL
+ * @encoder: encoder determining the type of port DPLL
+ *
+ * Update the active DPLL for the given @crtc/@encoder in @crtc's atomic state,
+ * from the port DPLLs reserved previously by intel_reserve_shared_dplls(). The
+ * DPLL selected will be based on the current mode of the encoder's port.
+ */
+void intel_update_active_dpll(struct intel_atomic_state *state,
+ struct intel_crtc *crtc,
+ struct intel_encoder *encoder)
+{
+ struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+ const struct intel_dpll_mgr *dpll_mgr = dev_priv->dpll_mgr;
+
+ if (WARN_ON(!dpll_mgr))
+ return;
+
+ dpll_mgr->update_active_dpll(state, crtc, encoder);
+}
+
/**
* intel_shared_dpll_dump_hw_state - write hw_state to dmesg
* @dev_priv: i915 drm device
diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
index 579f2ceafba3..1668f8116908 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
@@ -346,6 +346,9 @@ void intel_release_shared_dplls(struct intel_atomic_state *state,
struct intel_crtc *crtc);
void icl_set_active_port_dpll(struct intel_crtc_state *crtc_state,
enum icl_port_dpll_id port_dpll_id);
+void intel_update_active_dpll(struct intel_atomic_state *state,
+ struct intel_crtc *crtc,
+ struct intel_encoder *encoder);
void intel_prepare_shared_dpll(const struct intel_crtc_state *crtc_state);
void intel_enable_shared_dpll(const struct intel_crtc_state *crtc_state);
void intel_disable_shared_dpll(const struct intel_crtc_state *crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
index 7190b57376d6..c8904911e841 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.c
+++ b/drivers/gpu/drm/i915/display/intel_tc.c
@@ -172,19 +172,12 @@ static bool icl_tc_phy_is_in_safe_mode(struct intel_digital_port *dig_port)
* display, USB, etc. As a result, handshaking through FIA is required around
* connect and disconnect to cleanly transfer ownership with the controller and
* set the type-C power state.
- *
- * We could opt to only do the connect flow when we actually try to use the AUX
- * channels or do a modeset, then immediately run the disconnect flow after
- * usage, but there are some implications on this for a dynamic environment:
- * things may go away or change behind our backs. So for now our driver is
- * always trying to acquire ownership of the controller as soon as it gets an
- * interrupt (or polls state and sees a port is connected) and only gives it
- * back when it sees a disconnect. Implementation of a more fine-grained model
- * will require a lot of coordination with user space and thorough testing for
- * the extra possible cases.
*/
-static void icl_tc_phy_connect(struct intel_digital_port *dig_port)
+static void icl_tc_phy_connect(struct intel_digital_port *dig_port,
+ int required_lanes)
{
+ int max_lanes;
+
if (!icl_tc_phy_status_complete(dig_port)) {
DRM_DEBUG_KMS("Port %s: PHY not ready\n",
dig_port->tc_port_name);
@@ -195,8 +188,9 @@ static void icl_tc_phy_connect(struct intel_digital_port *dig_port)
!WARN_ON(dig_port->tc_legacy_port))
goto out_set_tbt_alt_mode;
+ max_lanes = intel_tc_port_fia_max_lane_count(dig_port);
if (dig_port->tc_legacy_port) {
- WARN_ON(intel_tc_port_fia_max_lane_count(dig_port) != 4);
+ WARN_ON(max_lanes != 4);
dig_port->tc_mode = TC_PORT_LEGACY;
return;
@@ -212,6 +206,13 @@ static void icl_tc_phy_connect(struct intel_digital_port *dig_port)
goto out_set_safe_mode;
}
+ if (max_lanes < required_lanes) {
+ DRM_DEBUG_KMS("Port %s: PHY max lanes %d < required lanes %d\n",
+ dig_port->tc_port_name,
+ max_lanes, required_lanes);
+ goto out_set_safe_mode;
+ }
+
dig_port->tc_mode = TC_PORT_DP_ALT;
return;
@@ -295,7 +296,8 @@ intel_tc_port_get_target_mode(struct intel_digital_port *dig_port)
TC_PORT_TBT_ALT;
}
-static void intel_tc_port_reset_mode(struct intel_digital_port *dig_port)
+static void intel_tc_port_reset_mode(struct intel_digital_port *dig_port,
+ int required_lanes)
{
struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
enum tc_port_mode old_tc_mode = dig_port->tc_mode;
@@ -303,7 +305,7 @@ static void intel_tc_port_reset_mode(struct intel_digital_port *dig_port)
intel_display_power_flush_work(dev_priv);
icl_tc_phy_disconnect(dig_port);
- icl_tc_phy_connect(dig_port);
+ icl_tc_phy_connect(dig_port, required_lanes);
DRM_DEBUG_KMS("Port %s: TC port mode reset (%s -> %s)\n",
dig_port->tc_port_name,
@@ -311,6 +313,14 @@ static void intel_tc_port_reset_mode(struct intel_digital_port *dig_port)
tc_port_mode_name(dig_port->tc_mode));
}
+static void
+intel_tc_port_link_init_refcount(struct intel_digital_port *dig_port,
+ int refcount)
+{
+ WARN_ON(dig_port->tc_link_refcount);
+ dig_port->tc_link_refcount = refcount;
+}
+
void intel_tc_port_sanitize(struct intel_digital_port *dig_port)
{
struct intel_encoder *encoder = &dig_port->base;
@@ -328,11 +338,13 @@ void intel_tc_port_sanitize(struct intel_digital_port *dig_port)
if (!icl_tc_phy_is_connected(dig_port))
DRM_DEBUG_KMS("Port %s: PHY disconnected with %d active link(s)\n",
dig_port->tc_port_name, active_links);
+ intel_tc_port_link_init_refcount(dig_port, active_links);
+
goto out;
}
if (dig_port->tc_legacy_port)
- icl_tc_phy_connect(dig_port);
+ icl_tc_phy_connect(dig_port, 1);
out:
DRM_DEBUG_KMS("Port %s: sanitize mode (%s)\n",
@@ -361,27 +373,60 @@ bool intel_tc_port_connected(struct intel_digital_port *dig_port)
{
bool is_connected;
- mutex_lock(&dig_port->tc_lock);
-
- if (intel_tc_port_needs_reset(dig_port))
- intel_tc_port_reset_mode(dig_port);
-
+ intel_tc_port_lock(dig_port);
is_connected = tc_port_live_status_mask(dig_port) &
BIT(dig_port->tc_mode);
-
- mutex_unlock(&dig_port->tc_lock);
+ intel_tc_port_unlock(dig_port);
return is_connected;
}
-void intel_tc_port_lock(struct intel_digital_port *dig_port)
+static void __intel_tc_port_lock(struct intel_digital_port *dig_port,
+ int required_lanes)
{
+ struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
+ intel_wakeref_t wakeref;
+
+ wakeref = intel_display_power_get(dev_priv, POWER_DOMAIN_DISPLAY_CORE);
+
mutex_lock(&dig_port->tc_lock);
- /* TODO: reset the TypeC port mode if needed */
+
+ if (!dig_port->tc_link_refcount &&
+ intel_tc_port_needs_reset(dig_port))
+ intel_tc_port_reset_mode(dig_port, required_lanes);
+
+ WARN_ON(dig_port->tc_lock_wakeref);
+ dig_port->tc_lock_wakeref = wakeref;
+}
+
+void intel_tc_port_lock(struct intel_digital_port *dig_port)
+{
+ __intel_tc_port_lock(dig_port, 1);
}
void intel_tc_port_unlock(struct intel_digital_port *dig_port)
{
+ struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
+ intel_wakeref_t wakeref = fetch_and_zero(&dig_port->tc_lock_wakeref);
+
+ mutex_unlock(&dig_port->tc_lock);
+
+ intel_display_power_put_async(dev_priv, POWER_DOMAIN_DISPLAY_CORE,
+ wakeref);
+}
+
+void intel_tc_port_get_link(struct intel_digital_port *dig_port,
+ int required_lanes)
+{
+ __intel_tc_port_lock(dig_port, required_lanes);
+ dig_port->tc_link_refcount++;
+ intel_tc_port_unlock(dig_port);
+}
+
+void intel_tc_port_put_link(struct intel_digital_port *dig_port)
+{
+ mutex_lock(&dig_port->tc_lock);
+ dig_port->tc_link_refcount--;
mutex_unlock(&dig_port->tc_lock);
}
@@ -399,4 +444,5 @@ void intel_tc_port_init(struct intel_digital_port *dig_port, bool is_legacy)
mutex_init(&dig_port->tc_lock);
dig_port->tc_legacy_port = is_legacy;
+ dig_port->tc_link_refcount = 0;
}
diff --git a/drivers/gpu/drm/i915/display/intel_tc.h b/drivers/gpu/drm/i915/display/intel_tc.h
index b5af2fe60b22..31af7be96070 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.h
+++ b/drivers/gpu/drm/i915/display/intel_tc.h
@@ -19,6 +19,9 @@ int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port);
void intel_tc_port_sanitize(struct intel_digital_port *dig_port);
void intel_tc_port_lock(struct intel_digital_port *dig_port);
void intel_tc_port_unlock(struct intel_digital_port *dig_port);
+void intel_tc_port_get_link(struct intel_digital_port *dig_port,
+ int required_lanes);
+void intel_tc_port_put_link(struct intel_digital_port *dig_port);
void intel_tc_port_init(struct intel_digital_port *dig_port, bool is_legacy);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 12a102e239c5..24c63ed45c6f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -115,6 +115,9 @@ struct intel_encoder {
int (*compute_config)(struct intel_encoder *,
struct intel_crtc_state *,
struct drm_connector_state *);
+ void (*update_prepare)(struct intel_atomic_state *,
+ struct intel_encoder *,
+ struct intel_crtc *);
void (*pre_pll_enable)(struct intel_encoder *,
const struct intel_crtc_state *,
const struct drm_connector_state *);
@@ -124,6 +127,9 @@ struct intel_encoder {
void (*enable)(struct intel_encoder *,
const struct intel_crtc_state *,
const struct drm_connector_state *);
+ void (*update_complete)(struct intel_atomic_state *,
+ struct intel_encoder *,
+ struct intel_crtc *);
void (*disable)(struct intel_encoder *,
const struct intel_crtc_state *,
const struct drm_connector_state *);
@@ -1234,6 +1240,8 @@ struct intel_digital_port {
enum aux_ch aux_ch;
enum intel_display_power_domain ddi_io_power_domain;
struct mutex tc_lock; /* protects the TypeC port mode */
+ intel_wakeref_t tc_lock_wakeref;
+ int tc_link_refcount;
bool tc_legacy_port:1;
char tc_port_name[8];
enum tc_port_mode tc_mode;
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related
* [PATCH v2 19/23] drm/i915/icl: Reserve all required PLLs for TypeC ports
From: Imre Deak @ 2019-06-20 14:05 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter
In-Reply-To: <20190620140600.11357-1-imre.deak@intel.com>
When enabling a TypeC port we need to reserve all the required PLLs for
it, the TBT PLL for TBT-alt and the MG PHY PLL for DP-alt/legacy sinks.
We can select the proper PLL for the current port mode from the reserved
PLLs only once we selected and locked down the port mode for the whole
duration of the port's active state. Resetting and locking down the port
mode can in turn happen only during the modeset commit phase once we
disabled the given port and the PLL it used.
To support the above reserve-and-select PLL semantic we store the
reserved PLLs along with their HW state in the CRTC state and provide a
way to select the active PLL from these. The selected PLL along with its
HW state will be pointed at by crtc_state->shared_dpll/dpll_hw_state as
in the case of other port types.
Besides reserving all required PLLs no functional changes.
v2:
- Fix releasing the ICL PLLs, not clearing the PLLs from the old
crtc_state.
- Init port_dpll to ICL_PORT_DPLL_DEFAULT closer to where port_dpll is
used for symmetry with the corresponding ICL_PORT_DPLL_MG_PHY init.
(Ville)
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
drivers/gpu/drm/i915/display/intel_display.c | 11 +-
drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 151 +++++++++++++-----
drivers/gpu/drm/i915/display/intel_dpll_mgr.h | 9 ++
drivers/gpu/drm/i915/intel_drv.h | 9 ++
4 files changed, 138 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 688137524179..93e3f568d7db 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -9958,6 +9958,7 @@ static void icelake_get_ddi_pll(struct drm_i915_private *dev_priv,
enum port port,
struct intel_crtc_state *pipe_config)
{
+ enum icl_port_dpll_id port_dpll_id;
enum intel_dpll_id id;
u32 temp;
@@ -9965,22 +9966,28 @@ static void icelake_get_ddi_pll(struct drm_i915_private *dev_priv,
temp = I915_READ(DPCLKA_CFGCR0_ICL) &
DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(port);
id = temp >> DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(port);
+ port_dpll_id = ICL_PORT_DPLL_DEFAULT;
} else if (intel_port_is_tc(dev_priv, port)) {
u32 clk_sel = I915_READ(DDI_CLK_SEL(port)) & DDI_CLK_SEL_MASK;
if (clk_sel == DDI_CLK_SEL_MG) {
id = icl_tc_port_to_pll_id(intel_port_to_tc(dev_priv,
port));
+ port_dpll_id = ICL_PORT_DPLL_MG_PHY;
} else {
WARN_ON(clk_sel < DDI_CLK_SEL_TBT_162);
id = DPLL_ID_ICL_TBTPLL;
+ port_dpll_id = ICL_PORT_DPLL_DEFAULT;
}
} else {
WARN(1, "Invalid port %x\n", port);
return;
}
- pipe_config->shared_dpll = intel_get_shared_dpll_by_id(dev_priv, id);
+ pipe_config->icl_port_dplls[port_dpll_id].pll =
+ intel_get_shared_dpll_by_id(dev_priv, id);
+
+ icl_set_active_port_dpll(pipe_config, port_dpll_id);
}
static void bxt_get_ddi_pll(struct drm_i915_private *dev_priv,
@@ -12119,6 +12126,8 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
saved_state->scaler_state = crtc_state->scaler_state;
saved_state->shared_dpll = crtc_state->shared_dpll;
saved_state->dpll_hw_state = crtc_state->dpll_hw_state;
+ memcpy(saved_state->icl_port_dplls, crtc_state->icl_port_dplls,
+ sizeof(saved_state->icl_port_dplls));
saved_state->crc_enabled = crtc_state->crc_enabled;
if (IS_G4X(dev_priv) ||
IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index 85c38eed93a8..a996a3fad48c 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -2856,34 +2856,79 @@ static bool icl_calc_mg_pll_state(struct intel_crtc_state *crtc_state,
return true;
}
+/**
+ * icl_set_active_port_dpll - select the active port DPLL for a given CRTC
+ * @crtc_state: state for the CRTC to select the DPLL for
+ * @port_dpll_id: the active @port_dpll_id to select
+ *
+ * Select the given @port_dpll_id instance from the DPLLs reserved for the
+ * CRTC.
+ */
+void icl_set_active_port_dpll(struct intel_crtc_state *crtc_state,
+ enum icl_port_dpll_id port_dpll_id)
+{
+ struct icl_port_dpll *port_dpll =
+ &crtc_state->icl_port_dplls[port_dpll_id];
+
+ crtc_state->shared_dpll = port_dpll->pll;
+ crtc_state->dpll_hw_state = port_dpll->hw_state;
+}
+
+static void icl_update_active_dpll(struct intel_atomic_state *state,
+ struct intel_crtc *crtc,
+ struct intel_encoder *encoder)
+{
+ struct intel_crtc_state *crtc_state =
+ intel_atomic_get_new_crtc_state(state, crtc);
+ struct intel_digital_port *primary_port;
+ enum icl_port_dpll_id port_dpll_id;
+
+ primary_port = encoder->type == INTEL_OUTPUT_DP_MST ?
+ enc_to_mst(&encoder->base)->primary :
+ enc_to_dig_port(&encoder->base);
+
+ switch (primary_port->tc_mode) {
+ case TC_PORT_TBT_ALT:
+ port_dpll_id = ICL_PORT_DPLL_DEFAULT;
+ break;
+ case TC_PORT_DP_ALT:
+ case TC_PORT_LEGACY:
+ port_dpll_id = ICL_PORT_DPLL_MG_PHY;
+ break;
+ }
+
+ icl_set_active_port_dpll(crtc_state, port_dpll_id);
+}
+
static bool icl_get_combo_phy_dpll(struct intel_atomic_state *state,
struct intel_crtc *crtc,
struct intel_encoder *encoder)
{
struct intel_crtc_state *crtc_state =
intel_atomic_get_new_crtc_state(state, crtc);
- struct intel_shared_dpll *pll;
+ struct icl_port_dpll *port_dpll =
+ &crtc_state->icl_port_dplls[ICL_PORT_DPLL_DEFAULT];
- if (!icl_calc_dpll_state(crtc_state, encoder,
- &crtc_state->dpll_hw_state)) {
+ if (!icl_calc_dpll_state(crtc_state, encoder, &port_dpll->hw_state)) {
DRM_DEBUG_KMS("Could not calculate combo PHY PLL state.\n");
return false;
}
- pll = intel_find_shared_dpll(state, crtc, &crtc_state->dpll_hw_state,
- DPLL_ID_ICL_DPLL0,
- DPLL_ID_ICL_DPLL1);
- if (!pll) {
+ port_dpll->pll = intel_find_shared_dpll(state, crtc,
+ &port_dpll->hw_state,
+ DPLL_ID_ICL_DPLL0,
+ DPLL_ID_ICL_DPLL1);
+ if (!port_dpll->pll) {
DRM_DEBUG_KMS("No combo PHY PLL found for port %c\n",
port_name(encoder->port));
return false;
}
intel_reference_shared_dpll(state, crtc,
- pll, &crtc_state->dpll_hw_state);
+ port_dpll->pll, &port_dpll->hw_state);
- crtc_state->shared_dpll = pll;
+ icl_update_active_dpll(state, crtc, encoder);
return true;
}
@@ -2895,49 +2940,55 @@ static bool icl_get_tc_phy_dplls(struct intel_atomic_state *state,
struct drm_i915_private *dev_priv = to_i915(state->base.dev);
struct intel_crtc_state *crtc_state =
intel_atomic_get_new_crtc_state(state, crtc);
- enum tc_port tc_port = intel_port_to_tc(dev_priv, encoder->port);
- struct intel_digital_port *dig_port;
- struct intel_shared_dpll *pll;
- enum intel_dpll_id min, max;
- bool ret;
-
- if (encoder->type == INTEL_OUTPUT_DP_MST)
- dig_port = enc_to_mst(&encoder->base)->primary;
- else
- dig_port = enc_to_dig_port(&encoder->base);
+ struct icl_port_dpll *port_dpll;
+ enum intel_dpll_id dpll_id;
- if (dig_port->tc_mode == TC_PORT_TBT_ALT) {
- min = DPLL_ID_ICL_TBTPLL;
- max = min;
- ret = icl_calc_dpll_state(crtc_state, encoder,
- &crtc_state->dpll_hw_state);
- } else {
- min = icl_tc_port_to_pll_id(tc_port);
- max = min;
- ret = icl_calc_mg_pll_state(crtc_state,
- &crtc_state->dpll_hw_state);
+ port_dpll = &crtc_state->icl_port_dplls[ICL_PORT_DPLL_DEFAULT];
+ if (!icl_calc_dpll_state(crtc_state, encoder, &port_dpll->hw_state)) {
+ DRM_DEBUG_KMS("Could not calculate TBT PLL state.\n");
+ return false;
}
- if (!ret) {
- DRM_DEBUG_KMS("Could not calculate PLL state.\n");
+ port_dpll->pll = intel_find_shared_dpll(state, crtc,
+ &port_dpll->hw_state,
+ DPLL_ID_ICL_TBTPLL,
+ DPLL_ID_ICL_TBTPLL);
+ if (!port_dpll->pll) {
+ DRM_DEBUG_KMS("No TBT-ALT PLL found\n");
return false;
}
+ intel_reference_shared_dpll(state, crtc,
+ port_dpll->pll, &port_dpll->hw_state);
- pll = intel_find_shared_dpll(state, crtc,
- &crtc_state->dpll_hw_state,
- min, max);
- if (!pll) {
- DRM_DEBUG_KMS("No PLL selected\n");
- return false;
+ port_dpll = &crtc_state->icl_port_dplls[ICL_PORT_DPLL_MG_PHY];
+ if (!icl_calc_mg_pll_state(crtc_state, &port_dpll->hw_state)) {
+ DRM_DEBUG_KMS("Could not calculate MG PHY PLL state.\n");
+ goto err_unreference_tbt_pll;
}
+ dpll_id = icl_tc_port_to_pll_id(intel_port_to_tc(dev_priv,
+ encoder->port));
+ port_dpll->pll = intel_find_shared_dpll(state, crtc,
+ &port_dpll->hw_state,
+ dpll_id,
+ dpll_id);
+ if (!port_dpll->pll) {
+ DRM_DEBUG_KMS("No MG PHY PLL found\n");
+ goto err_unreference_tbt_pll;
+ }
intel_reference_shared_dpll(state, crtc,
- pll, &crtc_state->dpll_hw_state);
+ port_dpll->pll, &port_dpll->hw_state);
- crtc_state->shared_dpll = pll;
+ icl_update_active_dpll(state, crtc, encoder);
return true;
+
+err_unreference_tbt_pll:
+ port_dpll = &crtc_state->icl_port_dplls[ICL_PORT_DPLL_DEFAULT];
+ intel_unreference_shared_dpll(state, crtc, port_dpll->pll);
+
+ return false;
}
static bool icl_get_dplls(struct intel_atomic_state *state,
@@ -2957,6 +3008,24 @@ static bool icl_get_dplls(struct intel_atomic_state *state,
return false;
}
+static void icl_put_dplls(struct intel_atomic_state *state,
+ struct intel_crtc *crtc)
+{
+ struct intel_crtc_state *crtc_state =
+ intel_atomic_get_old_crtc_state(state, crtc);
+ enum icl_port_dpll_id id;
+
+ for (id = ICL_PORT_DPLL_DEFAULT; id < ICL_PORT_DPLL_COUNT; id++) {
+ struct icl_port_dpll *port_dpll =
+ &crtc_state->icl_port_dplls[id];
+
+ if (!port_dpll->pll)
+ continue;
+
+ intel_unreference_shared_dpll(state, crtc, port_dpll->pll);
+ }
+}
+
static bool mg_pll_get_hw_state(struct drm_i915_private *dev_priv,
struct intel_shared_dpll *pll,
struct intel_dpll_hw_state *hw_state)
@@ -3330,7 +3399,7 @@ static const struct dpll_info icl_plls[] = {
static const struct intel_dpll_mgr icl_pll_mgr = {
.dpll_info = icl_plls,
.get_dplls = icl_get_dplls,
- .put_dplls = intel_put_dpll,
+ .put_dplls = icl_put_dplls,
.dump_hw_state = icl_dump_hw_state,
};
@@ -3343,7 +3412,7 @@ static const struct dpll_info ehl_plls[] = {
static const struct intel_dpll_mgr ehl_pll_mgr = {
.dpll_info = ehl_plls,
.get_dplls = icl_get_dplls,
- .put_dplls = intel_put_dpll,
+ .put_dplls = icl_put_dplls,
.dump_hw_state = icl_dump_hw_state,
};
diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
index 16ddab138574..579f2ceafba3 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
@@ -141,6 +141,13 @@ enum intel_dpll_id {
};
#define I915_NUM_PLLS 7
+enum icl_port_dpll_id {
+ ICL_PORT_DPLL_DEFAULT,
+ ICL_PORT_DPLL_MG_PHY,
+
+ ICL_PORT_DPLL_COUNT,
+};
+
struct intel_dpll_hw_state {
/* i9xx, pch plls */
u32 dpll;
@@ -337,6 +344,8 @@ bool intel_reserve_shared_dplls(struct intel_atomic_state *state,
struct intel_encoder *encoder);
void intel_release_shared_dplls(struct intel_atomic_state *state,
struct intel_crtc *crtc);
+void icl_set_active_port_dpll(struct intel_crtc_state *crtc_state,
+ enum icl_port_dpll_id port_dpll_id);
void intel_prepare_shared_dpll(const struct intel_crtc_state *crtc_state);
void intel_enable_shared_dpll(const struct intel_crtc_state *crtc_state);
void intel_disable_shared_dpll(const struct intel_crtc_state *crtc_state);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d9e7d011ed4a..12a102e239c5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -812,6 +812,15 @@ struct intel_crtc_state {
/* Actual register state of the dpll, for shared dpll cross-checking. */
struct intel_dpll_hw_state dpll_hw_state;
+ /*
+ * ICL reserved DPLLs for the CRTC/port. The active PLL is selected by
+ * setting shared_dpll and dpll_hw_state to one of these reserved ones.
+ */
+ struct icl_port_dpll {
+ struct intel_shared_dpll *pll;
+ struct intel_dpll_hw_state hw_state;
+ } icl_port_dplls[ICL_PORT_DPLL_COUNT];
+
/* DSI PLL registers */
struct {
u32 ctrl, div;
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related
* [PATCH v2 18/23] drm/i915/icl: Split getting the DPLLs to port type specific functions
From: Imre Deak @ 2019-06-20 14:05 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter
In-Reply-To: <20190620140600.11357-1-imre.deak@intel.com>
For clarity factor out the combo PHY and TypeC PHY specific code from
icl_get_dplls() into their own functions.
No functional changes.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 100 ++++++++++++------
1 file changed, 66 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index 14bbab45836d..85c38eed93a8 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -2856,51 +2856,66 @@ static bool icl_calc_mg_pll_state(struct intel_crtc_state *crtc_state,
return true;
}
-static bool icl_get_dplls(struct intel_atomic_state *state,
- struct intel_crtc *crtc,
- struct intel_encoder *encoder)
+static bool icl_get_combo_phy_dpll(struct intel_atomic_state *state,
+ struct intel_crtc *crtc,
+ struct intel_encoder *encoder)
+{
+ struct intel_crtc_state *crtc_state =
+ intel_atomic_get_new_crtc_state(state, crtc);
+ struct intel_shared_dpll *pll;
+
+ if (!icl_calc_dpll_state(crtc_state, encoder,
+ &crtc_state->dpll_hw_state)) {
+ DRM_DEBUG_KMS("Could not calculate combo PHY PLL state.\n");
+
+ return false;
+ }
+
+ pll = intel_find_shared_dpll(state, crtc, &crtc_state->dpll_hw_state,
+ DPLL_ID_ICL_DPLL0,
+ DPLL_ID_ICL_DPLL1);
+ if (!pll) {
+ DRM_DEBUG_KMS("No combo PHY PLL found for port %c\n",
+ port_name(encoder->port));
+ return false;
+ }
+
+ intel_reference_shared_dpll(state, crtc,
+ pll, &crtc_state->dpll_hw_state);
+
+ crtc_state->shared_dpll = pll;
+
+ return true;
+}
+
+static bool icl_get_tc_phy_dplls(struct intel_atomic_state *state,
+ struct intel_crtc *crtc,
+ struct intel_encoder *encoder)
{
struct drm_i915_private *dev_priv = to_i915(state->base.dev);
struct intel_crtc_state *crtc_state =
intel_atomic_get_new_crtc_state(state, crtc);
- struct intel_digital_port *intel_dig_port;
+ enum tc_port tc_port = intel_port_to_tc(dev_priv, encoder->port);
+ struct intel_digital_port *dig_port;
struct intel_shared_dpll *pll;
- enum port port = encoder->port;
enum intel_dpll_id min, max;
bool ret;
- if (intel_port_is_combophy(dev_priv, port)) {
- min = DPLL_ID_ICL_DPLL0;
- max = DPLL_ID_ICL_DPLL1;
+ if (encoder->type == INTEL_OUTPUT_DP_MST)
+ dig_port = enc_to_mst(&encoder->base)->primary;
+ else
+ dig_port = enc_to_dig_port(&encoder->base);
+
+ if (dig_port->tc_mode == TC_PORT_TBT_ALT) {
+ min = DPLL_ID_ICL_TBTPLL;
+ max = min;
ret = icl_calc_dpll_state(crtc_state, encoder,
&crtc_state->dpll_hw_state);
- } else if (intel_port_is_tc(dev_priv, port)) {
- if (encoder->type == INTEL_OUTPUT_DP_MST) {
- struct intel_dp_mst_encoder *mst_encoder;
-
- mst_encoder = enc_to_mst(&encoder->base);
- intel_dig_port = mst_encoder->primary;
- } else {
- intel_dig_port = enc_to_dig_port(&encoder->base);
- }
-
- if (intel_dig_port->tc_mode == TC_PORT_TBT_ALT) {
- min = DPLL_ID_ICL_TBTPLL;
- max = min;
- ret = icl_calc_dpll_state(crtc_state, encoder,
- &crtc_state->dpll_hw_state);
- } else {
- enum tc_port tc_port;
-
- tc_port = intel_port_to_tc(dev_priv, port);
- min = icl_tc_port_to_pll_id(tc_port);
- max = min;
- ret = icl_calc_mg_pll_state(crtc_state,
- &crtc_state->dpll_hw_state);
- }
} else {
- MISSING_CASE(port);
- return false;
+ min = icl_tc_port_to_pll_id(tc_port);
+ max = min;
+ ret = icl_calc_mg_pll_state(crtc_state,
+ &crtc_state->dpll_hw_state);
}
if (!ret) {
@@ -2925,6 +2940,23 @@ static bool icl_get_dplls(struct intel_atomic_state *state,
return true;
}
+static bool icl_get_dplls(struct intel_atomic_state *state,
+ struct intel_crtc *crtc,
+ struct intel_encoder *encoder)
+{
+ struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+ enum port port = encoder->port;
+
+ if (intel_port_is_combophy(dev_priv, port))
+ return icl_get_combo_phy_dpll(state, crtc, encoder);
+ else if (intel_port_is_tc(dev_priv, port))
+ return icl_get_tc_phy_dplls(state, crtc, encoder);
+
+ MISSING_CASE(port);
+
+ return false;
+}
+
static bool mg_pll_get_hw_state(struct drm_i915_private *dev_priv,
struct intel_shared_dpll *pll,
struct intel_dpll_hw_state *hw_state)
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related
* Re: [dpdk-dev] [PATCH] cryptodev: add ff_disable field in cryptodev config
From: Akhil Goyal @ 2019-06-20 14:06 UTC (permalink / raw)
To: Anoob Joseph, Declan Doherty, Fiona Trahe, Pablo de Lara,
dev@dpdk.org
Cc: Jerin Jacob, Narayana Prasad
In-Reply-To: <1559575528-5363-1-git-send-email-anoobj@marvell.com>
>
> Adding a new field, ff_disable, to allow applications to control the
> features enabled on the crypto device. This would allow for efficient
> usage of HW/SW offloads by disabling the features not required by the
> application.
>
> Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> ---
Acked-by: Akhil Goyal <akhil.goyal@nxp.com>
^ permalink raw reply
* [PATCH v2 17/23] drm/i915: Sanitize the shared DPLL find/reference interface
From: Imre Deak @ 2019-06-20 14:05 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter
In-Reply-To: <20190620140600.11357-1-imre.deak@intel.com>
Pass the PLL HW state to the PLL find/reference functions making it
clearer what is their input. Also pass to these the atomic state and the
CRTC object instead of the CRTC state, since they don't require the
latter.
Move setting the PLL in the crtc_state to the get_dpll() hook, which
is the more logical place for this, where the related PLL HW state was also
set.
This refactoring is also a preparation for a follow-up patch that will
have to find/reference multiple PLLs.
No functional changes.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 107 ++++++++++++------
1 file changed, 70 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index 3fbc975851fa..14bbab45836d 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -244,17 +244,18 @@ void intel_disable_shared_dpll(const struct intel_crtc_state *crtc_state)
}
static struct intel_shared_dpll *
-intel_find_shared_dpll(struct intel_crtc_state *crtc_state,
+intel_find_shared_dpll(struct intel_atomic_state *state,
+ const struct intel_crtc *crtc,
+ const struct intel_dpll_hw_state *pll_state,
enum intel_dpll_id range_min,
enum intel_dpll_id range_max)
{
- struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
struct intel_shared_dpll *pll, *unused_pll = NULL;
struct intel_shared_dpll_state *shared_dpll;
enum intel_dpll_id i;
- shared_dpll = intel_atomic_get_shared_dpll_state(crtc_state->base.state);
+ shared_dpll = intel_atomic_get_shared_dpll_state(&state->base);
for (i = range_min; i <= range_max; i++) {
pll = &dev_priv->shared_dplls[i];
@@ -266,9 +267,9 @@ intel_find_shared_dpll(struct intel_crtc_state *crtc_state,
continue;
}
- if (memcmp(&crtc_state->dpll_hw_state,
+ if (memcmp(pll_state,
&shared_dpll[i].hw_state,
- sizeof(crtc_state->dpll_hw_state)) == 0) {
+ sizeof(*pll_state)) == 0) {
DRM_DEBUG_KMS("[CRTC:%d:%s] sharing existing %s (crtc mask 0x%08x, active %x)\n",
crtc->base.base.id, crtc->base.name,
pll->info->name,
@@ -290,20 +291,19 @@ intel_find_shared_dpll(struct intel_crtc_state *crtc_state,
}
static void
-intel_reference_shared_dpll(struct intel_shared_dpll *pll,
- struct intel_crtc_state *crtc_state)
+intel_reference_shared_dpll(struct intel_atomic_state *state,
+ const struct intel_crtc *crtc,
+ const struct intel_shared_dpll *pll,
+ const struct intel_dpll_hw_state *pll_state)
{
struct intel_shared_dpll_state *shared_dpll;
- struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
const enum intel_dpll_id id = pll->info->id;
- shared_dpll = intel_atomic_get_shared_dpll_state(crtc_state->base.state);
+ shared_dpll = intel_atomic_get_shared_dpll_state(&state->base);
if (shared_dpll[id].crtc_mask == 0)
- shared_dpll[id].hw_state =
- crtc_state->dpll_hw_state;
+ shared_dpll[id].hw_state = *pll_state;
- crtc_state->shared_dpll = pll;
DRM_DEBUG_DRIVER("using %s for pipe %c\n", pll->info->name,
pipe_name(crtc->pipe));
@@ -463,7 +463,8 @@ static bool ibx_get_dpll(struct intel_atomic_state *state,
crtc->base.base.id, crtc->base.name,
pll->info->name);
} else {
- pll = intel_find_shared_dpll(crtc_state,
+ pll = intel_find_shared_dpll(state, crtc,
+ &crtc_state->dpll_hw_state,
DPLL_ID_PCH_PLL_A,
DPLL_ID_PCH_PLL_B);
}
@@ -472,7 +473,10 @@ static bool ibx_get_dpll(struct intel_atomic_state *state,
return false;
/* reference the pll */
- intel_reference_shared_dpll(pll, crtc_state);
+ intel_reference_shared_dpll(state, crtc,
+ pll, &crtc_state->dpll_hw_state);
+
+ crtc_state->shared_dpll = pll;
return true;
}
@@ -791,8 +795,12 @@ hsw_ddi_calculate_wrpll(int clock /* in Hz */,
*r2_out = best.r2;
}
-static struct intel_shared_dpll *hsw_ddi_hdmi_get_dpll(struct intel_crtc_state *crtc_state)
+static struct intel_shared_dpll *
+hsw_ddi_hdmi_get_dpll(struct intel_atomic_state *state,
+ struct intel_crtc *crtc)
{
+ struct intel_crtc_state *crtc_state =
+ intel_atomic_get_new_crtc_state(state, crtc);
struct intel_shared_dpll *pll;
u32 val;
unsigned int p, n2, r2;
@@ -805,7 +813,8 @@ static struct intel_shared_dpll *hsw_ddi_hdmi_get_dpll(struct intel_crtc_state *
crtc_state->dpll_hw_state.wrpll = val;
- pll = intel_find_shared_dpll(crtc_state,
+ pll = intel_find_shared_dpll(state, crtc,
+ &crtc_state->dpll_hw_state,
DPLL_ID_WRPLL1, DPLL_ID_WRPLL2);
if (!pll)
@@ -857,7 +866,7 @@ static bool hsw_get_dpll(struct intel_atomic_state *state,
sizeof(crtc_state->dpll_hw_state));
if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
- pll = hsw_ddi_hdmi_get_dpll(crtc_state);
+ pll = hsw_ddi_hdmi_get_dpll(state, crtc);
} else if (intel_crtc_has_dp_encoder(crtc_state)) {
pll = hsw_ddi_dp_get_dpll(crtc_state);
} else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_ANALOG)) {
@@ -867,7 +876,8 @@ static bool hsw_get_dpll(struct intel_atomic_state *state,
crtc_state->dpll_hw_state.spll =
SPLL_PLL_ENABLE | SPLL_FREQ_1350MHz | SPLL_REF_MUXED_SSC;
- pll = intel_find_shared_dpll(crtc_state,
+ pll = intel_find_shared_dpll(state, crtc,
+ &crtc_state->dpll_hw_state,
DPLL_ID_SPLL, DPLL_ID_SPLL);
} else {
return false;
@@ -876,7 +886,10 @@ static bool hsw_get_dpll(struct intel_atomic_state *state,
if (!pll)
return false;
- intel_reference_shared_dpll(pll, crtc_state);
+ intel_reference_shared_dpll(state, crtc,
+ pll, &crtc_state->dpll_hw_state);
+
+ crtc_state->shared_dpll = pll;
return true;
}
@@ -1437,17 +1450,22 @@ static bool skl_get_dpll(struct intel_atomic_state *state,
}
if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP))
- pll = intel_find_shared_dpll(crtc_state,
+ pll = intel_find_shared_dpll(state, crtc,
+ &crtc_state->dpll_hw_state,
DPLL_ID_SKL_DPLL0,
DPLL_ID_SKL_DPLL0);
else
- pll = intel_find_shared_dpll(crtc_state,
+ pll = intel_find_shared_dpll(state, crtc,
+ &crtc_state->dpll_hw_state,
DPLL_ID_SKL_DPLL1,
DPLL_ID_SKL_DPLL3);
if (!pll)
return false;
- intel_reference_shared_dpll(pll, crtc_state);
+ intel_reference_shared_dpll(state, crtc,
+ pll, &crtc_state->dpll_hw_state);
+
+ crtc_state->shared_dpll = pll;
return true;
}
@@ -1880,7 +1898,10 @@ static bool bxt_get_dpll(struct intel_atomic_state *state,
DRM_DEBUG_KMS("[CRTC:%d:%s] using pre-allocated %s\n",
crtc->base.base.id, crtc->base.name, pll->info->name);
- intel_reference_shared_dpll(pll, crtc_state);
+ intel_reference_shared_dpll(state, crtc,
+ pll, &crtc_state->dpll_hw_state);
+
+ crtc_state->shared_dpll = pll;
return true;
}
@@ -2395,7 +2416,8 @@ static bool cnl_get_dpll(struct intel_atomic_state *state,
return false;
}
- pll = intel_find_shared_dpll(crtc_state,
+ pll = intel_find_shared_dpll(state, crtc,
+ &crtc_state->dpll_hw_state,
DPLL_ID_SKL_DPLL0,
DPLL_ID_SKL_DPLL2);
if (!pll) {
@@ -2403,7 +2425,10 @@ static bool cnl_get_dpll(struct intel_atomic_state *state,
return false;
}
- intel_reference_shared_dpll(pll, crtc_state);
+ intel_reference_shared_dpll(state, crtc,
+ pll, &crtc_state->dpll_hw_state);
+
+ crtc_state->shared_dpll = pll;
return true;
}
@@ -2545,7 +2570,8 @@ static bool icl_calc_tbt_pll(struct intel_crtc_state *crtc_state,
}
static bool icl_calc_dpll_state(struct intel_crtc_state *crtc_state,
- struct intel_encoder *encoder)
+ struct intel_encoder *encoder,
+ struct intel_dpll_hw_state *pll_state)
{
struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
u32 cfgcr0, cfgcr1;
@@ -2572,11 +2598,10 @@ static bool icl_calc_dpll_state(struct intel_crtc_state *crtc_state,
DPLL_CFGCR1_PDIV(pll_params.pdiv) |
DPLL_CFGCR1_CENTRAL_FREQ_8400;
- memset(&crtc_state->dpll_hw_state, 0,
- sizeof(crtc_state->dpll_hw_state));
+ memset(pll_state, 0, sizeof(*pll_state));
- crtc_state->dpll_hw_state.cfgcr0 = cfgcr0;
- crtc_state->dpll_hw_state.cfgcr1 = cfgcr1;
+ pll_state->cfgcr0 = cfgcr0;
+ pll_state->cfgcr1 = cfgcr1;
return true;
}
@@ -2666,10 +2691,10 @@ static bool icl_mg_pll_find_divisors(int clock_khz, bool is_dp, bool use_ssc,
* The specification for this function uses real numbers, so the math had to be
* adapted to integer-only calculation, that's why it looks so different.
*/
-static bool icl_calc_mg_pll_state(struct intel_crtc_state *crtc_state)
+static bool icl_calc_mg_pll_state(struct intel_crtc_state *crtc_state,
+ struct intel_dpll_hw_state *pll_state)
{
struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
- struct intel_dpll_hw_state *pll_state = &crtc_state->dpll_hw_state;
int refclk_khz = dev_priv->cdclk.hw.ref;
int clock = crtc_state->port_clock;
u32 dco_khz, m1div, m2div_int, m2div_rem, m2div_frac;
@@ -2847,7 +2872,8 @@ static bool icl_get_dplls(struct intel_atomic_state *state,
if (intel_port_is_combophy(dev_priv, port)) {
min = DPLL_ID_ICL_DPLL0;
max = DPLL_ID_ICL_DPLL1;
- ret = icl_calc_dpll_state(crtc_state, encoder);
+ ret = icl_calc_dpll_state(crtc_state, encoder,
+ &crtc_state->dpll_hw_state);
} else if (intel_port_is_tc(dev_priv, port)) {
if (encoder->type == INTEL_OUTPUT_DP_MST) {
struct intel_dp_mst_encoder *mst_encoder;
@@ -2861,14 +2887,16 @@ static bool icl_get_dplls(struct intel_atomic_state *state,
if (intel_dig_port->tc_mode == TC_PORT_TBT_ALT) {
min = DPLL_ID_ICL_TBTPLL;
max = min;
- ret = icl_calc_dpll_state(crtc_state, encoder);
+ ret = icl_calc_dpll_state(crtc_state, encoder,
+ &crtc_state->dpll_hw_state);
} else {
enum tc_port tc_port;
tc_port = intel_port_to_tc(dev_priv, port);
min = icl_tc_port_to_pll_id(tc_port);
max = min;
- ret = icl_calc_mg_pll_state(crtc_state);
+ ret = icl_calc_mg_pll_state(crtc_state,
+ &crtc_state->dpll_hw_state);
}
} else {
MISSING_CASE(port);
@@ -2881,13 +2909,18 @@ static bool icl_get_dplls(struct intel_atomic_state *state,
}
- pll = intel_find_shared_dpll(crtc_state, min, max);
+ pll = intel_find_shared_dpll(state, crtc,
+ &crtc_state->dpll_hw_state,
+ min, max);
if (!pll) {
DRM_DEBUG_KMS("No PLL selected\n");
return false;
}
- intel_reference_shared_dpll(pll, crtc_state);
+ intel_reference_shared_dpll(state, crtc,
+ pll, &crtc_state->dpll_hw_state);
+
+ crtc_state->shared_dpll = pll;
return true;
}
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.