date:20221005

On Wed, Oct 05, 2022 at 11:48:41PM +0200, Jason A. Donenfeld wrote:
> Rather than truncate a 32-bit value to a 16-bit value or an 8-bit value,
> simply use the get_random_{u8,u16}() functions, which are faster than
> wasting the additional bytes from a 32-bit value.
> 
> Signed-off-by: Jason A. Donenfeld 

Same question about "mechanism of transformation".

> diff --git a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c 
> b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
> index ddfe9208529a..ac452a0111a9 100644
> --- a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
> +++ b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
> @@ -1467,7 +1467,7 @@ static void make_established(struct sock *sk, u32 
> snd_isn, unsigned int opt)
>   tp->write_seq = snd_isn;
>   tp->snd_nxt = snd_isn;
>   tp->snd_una = snd_isn;
> - inet_sk(sk)->inet_id = prandom_u32();
> + inet_sk(sk)->inet_id = get_random_u16();
>   assign_rxopt(sk, opt);
>  
>   if (tp->rcv_wnd > (RCV_BUFSIZ_M << 10))

This one I had to go look at -- inet_id is u16, so yeah. :)

> diff --git a/lib/test_vmalloc.c b/lib/test_vmalloc.c
> index 56ffaa8dd3f6..0131ed2cd1bd 100644
> --- a/lib/test_vmalloc.c
> +++ b/lib/test_vmalloc.c
> @@ -80,7 +80,7 @@ static int random_size_align_alloc_test(void)
>   int i;
>  
>   for (i = 0; i < test_loop_count; i++) {
> - rnd = prandom_u32();
> + rnd = get_random_u8();
>  
>   /*
>* Maximum 1024 pages, if PAGE_SIZE is 4096.

This wasn't obvious either, but it looks like it's because it never
consumes more than u8?

> diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
> index 7981be526f26..57c7686ac485 100644
> --- a/net/netfilter/nf_nat_core.c
> +++ b/net/netfilter/nf_nat_core.c
> @@ -468,7 +468,7 @@ static void nf_nat_l4proto_unique_tuple(struct 
> nf_conntrack_tuple *tuple,
>   if (range->flags & NF_NAT_RANGE_PROTO_OFFSET)
>   off = (ntohs(*keyptr) - ntohs(range->base_proto.all));
>   else
> - off = prandom_u32();
> + off = get_random_u16();
>  
>   attempts = range_size;

Yup, u16 off;

> diff --git a/net/sched/sch_sfb.c b/net/sched/sch_sfb.c
> index 2829455211f8..7eb70acb4d58 100644
> --- a/net/sched/sch_sfb.c
> +++ b/net/sched/sch_sfb.c
> @@ -379,7 +379,7 @@ static int sfb_enqueue(struct sk_buff *skb, struct Qdisc 
> *sch,
>   goto enqueue;
>   }
>  
> - r = prandom_u32() & SFB_MAX_PROB;
> + r = get_random_u16() & SFB_MAX_PROB;
>  
>   if (unlikely(r < p_min)) {
>   if (unlikely(p_min > SFB_MAX_PROB / 2)) {

include/uapi/linux/pkt_sched.h:#define SFB_MAX_PROB 0x

Reviewed-by: Kees Cook 

-- 
Kees Cook

Re: [PATCH v1 5/5] prandom: remove unused functions

On Wed, Oct 05, 2022 at 11:48:44PM +0200, Jason A. Donenfeld wrote:
> With no callers left of prandom_u32() and prandom_bytes(), remove these
> deprecated wrappers.
> 
> Signed-off-by: Jason A. Donenfeld 

Reviewed-by: Kees Cook 

-- 
Kees Cook

[PATCH v1 5/5] prandom: remove unused functions

With no callers left of prandom_u32() and prandom_bytes(), remove these
deprecated wrappers.

Signed-off-by: Jason A. Donenfeld 
---
 include/linux/prandom.h | 12 
 1 file changed, 12 deletions(-)

diff --git a/include/linux/prandom.h b/include/linux/prandom.h
index 78db003bc290..e0a0759dd09c 100644
--- a/include/linux/prandom.h
+++ b/include/linux/prandom.h
@@ -12,18 +12,6 @@
 #include 
 #include 
 
-/* Deprecated: use get_random_u32 instead. */
-static inline u32 prandom_u32(void)
-{
-   return get_random_u32();
-}
-
-/* Deprecated: use get_random_bytes instead. */
-static inline void prandom_bytes(void *buf, size_t nbytes)
-{
-   return get_random_bytes(buf, nbytes);
-}
-
 struct rnd_state {
__u32 s1, s2, s3, s4;
 };
-- 
2.37.3

Re: [PATCH v1 4/5] treewide: use get_random_bytes when possible

On Wed, Oct 05, 2022 at 11:48:43PM +0200, Jason A. Donenfeld wrote:
> The prandom_bytes() function has been a deprecated inline wrapper around
> get_random_bytes() for several releases now, and compiles down to the
> exact same code. Replace the deprecated wrapper with a direct call to
> the real function.

Global search/replace matches. :)

Reviewed-by: Kees Cook 

-- 
Kees Cook

Re: [PATCH v1 4/5] treewide: use get_random_bytes when possible

On Wed, Oct 05, 2022 at 11:48:43PM +0200, Jason A. Donenfeld wrote:
> The prandom_bytes() function has been a deprecated inline wrapper around
> get_random_bytes() for several releases now, and compiles down to the
> exact same code. Replace the deprecated wrapper with a direct call to
> the real function.
> 
> Signed-off-by: Jason A. Donenfeld 

Global search/replace matches. :)

Reviewed-by: Kees Cook 

-- 
Kees Cook

[PATCH v1 4/5] treewide: use get_random_bytes when possible

The prandom_bytes() function has been a deprecated inline wrapper around
get_random_bytes() for several releases now, and compiles down to the
exact same code. Replace the deprecated wrapper with a direct call to
the real function.

Signed-off-by: Jason A. Donenfeld 
---
 arch/powerpc/crypto/crc-vpmsum_test.c   |  2 +-
 block/blk-crypto-fallback.c |  2 +-
 crypto/async_tx/raid6test.c |  2 +-
 drivers/dma/dmatest.c   |  2 +-
 drivers/mtd/nand/raw/nandsim.c  |  2 +-
 drivers/mtd/tests/mtd_nandecctest.c |  2 +-
 drivers/mtd/tests/speedtest.c   |  2 +-
 drivers/mtd/tests/stresstest.c  |  2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt.c   |  2 +-
 drivers/net/ethernet/rocker/rocker_main.c   |  2 +-
 drivers/net/wireguard/selftest/allowedips.c | 12 ++--
 fs/ubifs/debug.c|  2 +-
 kernel/kcsan/selftest.c |  2 +-
 lib/random32.c  |  2 +-
 lib/test_objagg.c   |  2 +-
 lib/uuid.c  |  2 +-
 net/ipv4/route.c|  2 +-
 net/mac80211/rc80211_minstrel_ht.c  |  2 +-
 net/sched/sch_pie.c |  2 +-
 19 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/arch/powerpc/crypto/crc-vpmsum_test.c 
b/arch/powerpc/crypto/crc-vpmsum_test.c
index c1c1ef9457fb..273c527868db 100644
--- a/arch/powerpc/crypto/crc-vpmsum_test.c
+++ b/arch/powerpc/crypto/crc-vpmsum_test.c
@@ -82,7 +82,7 @@ static int __init crc_test_init(void)
 
if (len <= offset)
continue;
-   prandom_bytes(data, len);
+   get_random_bytes(data, len);
len -= offset;
 
crypto_shash_update(crct10dif_shash, data+offset, len);
diff --git a/block/blk-crypto-fallback.c b/block/blk-crypto-fallback.c
index 621abd1b0e4d..ad9844c5b40c 100644
--- a/block/blk-crypto-fallback.c
+++ b/block/blk-crypto-fallback.c
@@ -539,7 +539,7 @@ static int blk_crypto_fallback_init(void)
if (blk_crypto_fallback_inited)
return 0;
 
-   prandom_bytes(blank_key, BLK_CRYPTO_MAX_KEY_SIZE);
+   get_random_bytes(blank_key, BLK_CRYPTO_MAX_KEY_SIZE);
 
err = bioset_init(&crypto_bio_split, 64, 0, 0);
if (err)
diff --git a/crypto/async_tx/raid6test.c b/crypto/async_tx/raid6test.c
index c9d218e53bcb..f74505f2baf0 100644
--- a/crypto/async_tx/raid6test.c
+++ b/crypto/async_tx/raid6test.c
@@ -37,7 +37,7 @@ static void makedata(int disks)
int i;
 
for (i = 0; i < disks; i++) {
-   prandom_bytes(page_address(data[i]), PAGE_SIZE);
+   get_random_bytes(page_address(data[i]), PAGE_SIZE);
dataptrs[i] = data[i];
dataoffs[i] = 0;
}
diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index 9fe2ae794316..ffe621695e47 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -312,7 +312,7 @@ static unsigned long dmatest_random(void)
 {
unsigned long buf;
 
-   prandom_bytes(&buf, sizeof(buf));
+   get_random_bytes(&buf, sizeof(buf));
return buf;
 }
 
diff --git a/drivers/mtd/nand/raw/nandsim.c b/drivers/mtd/nand/raw/nandsim.c
index 4bdaf4aa7007..c941a5a41ea6 100644
--- a/drivers/mtd/nand/raw/nandsim.c
+++ b/drivers/mtd/nand/raw/nandsim.c
@@ -1393,7 +1393,7 @@ static int ns_do_read_error(struct nandsim *ns, int num)
unsigned int page_no = ns->regs.row;
 
if (ns_read_error(page_no)) {
-   prandom_bytes(ns->buf.byte, num);
+   get_random_bytes(ns->buf.byte, num);
NS_WARN("simulating read error in page %u\n", page_no);
return 1;
}
diff --git a/drivers/mtd/tests/mtd_nandecctest.c 
b/drivers/mtd/tests/mtd_nandecctest.c
index 1c7201b0f372..440988562cfd 100644
--- a/drivers/mtd/tests/mtd_nandecctest.c
+++ b/drivers/mtd/tests/mtd_nandecctest.c
@@ -266,7 +266,7 @@ static int nand_ecc_test_run(const size_t size)
goto error;
}
 
-   prandom_bytes(correct_data, size);
+   get_random_bytes(correct_data, size);
ecc_sw_hamming_calculate(correct_data, size, correct_ecc, sm_order);
for (i = 0; i < ARRAY_SIZE(nand_ecc_test); i++) {
nand_ecc_test[i].prepare(error_data, error_ecc,
diff --git a/drivers/mtd/tests/speedtest.c b/drivers/mtd/tests/speedtest.c
index c9ec7086bfa1..075bce32caa5 100644
--- a/drivers/mtd/tests/speedtest.c
+++ b/drivers/mtd/tests/speedtest.c
@@ -223,7 +223,7 @@ static int __init mtd_speedtest_init(void)
if (!iobuf)
goto out;
 
-   prandom_bytes(iobuf, mtd->erasesize);
+   get_random_bytes(iobuf, mtd->erasesize);
 
bbt = kzalloc(ebcnt, GFP_KERNEL);
if (!bbt)
diff --git a/drivers/mtd/tests/stresstest.c b/drivers/mtd/tests/stresstest.

[PATCH v1 3/5] treewide: use get_random_u32() when possible

The prandom_u32() function has been a deprecated inline wrapper around
get_random_u32() for several releases now, and compiles down to the
exact same code. Replace the deprecated wrapper with a direct call to
the real function.

Signed-off-by: Jason A. Donenfeld 
---
 Documentation/networking/filter.rst|  2 +-
 drivers/infiniband/hw/cxgb4/cm.c   |  4 ++--
 drivers/infiniband/hw/hfi1/tid_rdma.c  |  2 +-
 drivers/infiniband/hw/mlx4/mad.c   |  2 +-
 drivers/infiniband/ulp/ipoib/ipoib_cm.c|  2 +-
 drivers/md/raid5-cache.c   |  2 +-
 drivers/mtd/nand/raw/nandsim.c |  2 +-
 drivers/net/bonding/bond_main.c|  2 +-
 drivers/net/ethernet/broadcom/cnic.c   |  2 +-
 .../chelsio/inline_crypto/chtls/chtls_cm.c |  2 +-
 drivers/net/ethernet/rocker/rocker_main.c  |  6 +++---
 .../net/wireless/marvell/mwifiex/cfg80211.c|  4 ++--
 .../net/wireless/microchip/wilc1000/cfg80211.c |  2 +-
 .../net/wireless/quantenna/qtnfmac/cfg80211.c  |  2 +-
 drivers/nvme/common/auth.c |  2 +-
 drivers/scsi/cxgbi/cxgb4i/cxgb4i.c |  4 ++--
 drivers/target/iscsi/cxgbit/cxgbit_cm.c|  2 +-
 drivers/thunderbolt/xdomain.c  |  2 +-
 drivers/video/fbdev/uvesafb.c  |  2 +-
 fs/exfat/inode.c   |  2 +-
 fs/ext2/ialloc.c   |  2 +-
 fs/ext4/ialloc.c   |  4 ++--
 fs/ext4/ioctl.c|  4 ++--
 fs/ext4/mmp.c  |  2 +-
 fs/f2fs/namei.c|  2 +-
 fs/fat/inode.c |  2 +-
 fs/nfsd/nfs4state.c|  4 ++--
 fs/ubifs/journal.c |  2 +-
 fs/xfs/libxfs/xfs_ialloc.c |  2 +-
 fs/xfs/xfs_icache.c|  2 +-
 fs/xfs/xfs_log.c   |  2 +-
 include/net/netfilter/nf_queue.h   |  2 +-
 include/net/red.h  |  2 +-
 include/net/sock.h |  2 +-
 kernel/kcsan/selftest.c|  2 +-
 lib/random32.c |  2 +-
 lib/reed_solomon/test_rslib.c  |  6 +++---
 lib/test_fprobe.c  |  2 +-
 lib/test_kprobes.c |  2 +-
 lib/test_rhashtable.c  |  6 +++---
 mm/shmem.c |  2 +-
 net/802/garp.c |  2 +-
 net/802/mrp.c  |  2 +-
 net/core/pktgen.c  |  4 ++--
 net/ipv4/tcp_cdg.c |  2 +-
 net/ipv4/udp.c |  2 +-
 net/ipv6/ip6_flowlabel.c   |  2 +-
 net/ipv6/output_core.c |  2 +-
 net/netfilter/ipvs/ip_vs_conn.c|  2 +-
 net/netfilter/xt_statistic.c   |  2 +-
 net/openvswitch/actions.c  |  2 +-
 net/rds/bind.c |  2 +-
 net/sched/sch_cake.c   |  2 +-
 net/sched/sch_netem.c  | 18 +-
 net/sunrpc/auth_gss/gss_krb5_wrap.c|  4 ++--
 net/sunrpc/xprt.c  |  2 +-
 net/unix/af_unix.c |  2 +-
 57 files changed, 79 insertions(+), 79 deletions(-)

diff --git a/Documentation/networking/filter.rst 
b/Documentation/networking/filter.rst
index 43cdc4d34745..f69da5074860 100644
--- a/Documentation/networking/filter.rst
+++ b/Documentation/networking/filter.rst
@@ -305,7 +305,7 @@ Possible BPF extensions are shown in the following table:
   vlan_tci  skb_vlan_tag_get(skb)
   vlan_availskb_vlan_tag_present(skb)
   vlan_tpid skb->vlan_proto
-  rand  prandom_u32()
+  rand  get_random_u32()
   ===   
=
 
 These extensions can also be prefixed with '#'.
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index 14392c942f49..499a425a3379 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -734,7 +734,7 @@ static int send_connect(struct c4iw_ep *ep)
   &ep->com.remote_addr;
int ret;
enum chip_type adapter_type = ep->com.dev->rdev.lldi.adapter_type;
-   u32 isn = (prandom_u32() & ~7UL) - 1;
+   u32 isn = (get_random_u32() & ~7UL) - 1;
struct net_device *netdev;
u64 params;
 
@@ -2469,7 +2469,7 @@ static int accept_cr(struct c4iw_ep *ep, struct sk_buff 
*skb,
}
 
if (!is_t4(adapter_type)) {
-

Re: [PATCH v1 1/5] treewide: use prandom_u32_max() when possible

On Wed, Oct 05, 2022 at 11:48:40PM +0200, Jason A. Donenfeld wrote:
> Rather than incurring a division or requesting too many random bytes for
> the given range, use the prandom_u32_max() function, which only takes
> the minimum required bytes from the RNG and avoids divisions.

Yes please!

Since this is a treewide patch, it's helpful for (me at least) doing
reviews to detail the mechanism of the transformation.

e.g. I imagine this could be done with something like Coccinelle and

@no_modulo@
expression E;
@@

-   (prandom_u32() % (E))
+   prandom_u32_max(E)

> diff --git a/drivers/mtd/ubi/debug.h b/drivers/mtd/ubi/debug.h
> index 118248a5d7d4..4236c799a47c 100644
> --- a/drivers/mtd/ubi/debug.h
> +++ b/drivers/mtd/ubi/debug.h
> @@ -73,7 +73,7 @@ static inline int ubi_dbg_is_bgt_disabled(const struct 
> ubi_device *ubi)
>  static inline int ubi_dbg_is_bitflip(const struct ubi_device *ubi)
>  {
>   if (ubi->dbg.emulate_bitflips)
> - return !(prandom_u32() % 200);
> + return !(prandom_u32_max(200));
>   return 0;
>  }
>  

Because some looks automated (why the parens?)

> @@ -393,14 +387,11 @@ static struct test_driver {
>  
>  static void shuffle_array(int *arr, int n)
>  {
> - unsigned int rnd;
>   int i, j;
>  
>   for (i = n - 1; i > 0; i--)  {
> - rnd = prandom_u32();
> -
>   /* Cut the range. */
> - j = rnd % i;
> + j = prandom_u32_max(i);
>  
>   /* Swap indexes. */
>   swap(arr[i], arr[j]);

And some by hand. :)

Reviewed-by: Kees Cook 

-- 
Kees Cook

[PATCH v1 2/5] treewide: use get_random_{u8,u16}() when possible

Rather than truncate a 32-bit value to a 16-bit value or an 8-bit value,
simply use the get_random_{u8,u16}() functions, which are faster than
wasting the additional bytes from a 32-bit value.

Signed-off-by: Jason A. Donenfeld 
---
 crypto/testmgr.c  | 8 
 drivers/media/common/v4l2-tpg/v4l2-tpg-core.c | 2 +-
 drivers/media/test-drivers/vivid/vivid-radio-rx.c | 4 ++--
 .../net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c   | 2 +-
 drivers/net/hamradio/baycom_epp.c | 2 +-
 drivers/net/hamradio/hdlcdrv.c| 2 +-
 drivers/net/hamradio/yam.c| 2 +-
 drivers/net/wireguard/selftest/allowedips.c   | 4 ++--
 drivers/scsi/lpfc/lpfc_hbadisc.c  | 6 +++---
 lib/test_vmalloc.c| 2 +-
 net/dccp/ipv4.c   | 4 ++--
 net/ipv4/datagram.c   | 2 +-
 net/ipv4/ip_output.c  | 2 +-
 net/ipv4/tcp_ipv4.c   | 4 ++--
 net/mac80211/scan.c   | 2 +-
 net/netfilter/nf_nat_core.c   | 4 ++--
 net/sched/sch_cake.c  | 6 +++---
 net/sched/sch_sfb.c   | 2 +-
 net/sctp/socket.c | 2 +-
 19 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index be45217acde4..981c637fa2ed 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -927,7 +927,7 @@ static void generate_random_bytes(u8 *buf, size_t count)
b = 0xff;
break;
default:
-   b = (u8)prandom_u32();
+   b = get_random_u8();
break;
}
memset(buf, b, count);
@@ -935,8 +935,8 @@ static void generate_random_bytes(u8 *buf, size_t count)
break;
case 2:
/* Ascending or descending bytes, plus optional mutations */
-   increment = (u8)prandom_u32();
-   b = (u8)prandom_u32();
+   increment = get_random_u8();
+   b = get_random_u8();
for (i = 0; i < count; i++, b += increment)
buf[i] = b;
mutate_buffer(buf, count);
@@ -944,7 +944,7 @@ static void generate_random_bytes(u8 *buf, size_t count)
default:
/* Fully random bytes */
for (i = 0; i < count; i++)
-   buf[i] = (u8)prandom_u32();
+   buf[i] = get_random_u8();
}
 }
 
diff --git a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c 
b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c
index 9b7bcdce6e44..303d02b1d71c 100644
--- a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c
+++ b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c
@@ -870,7 +870,7 @@ static void precalculate_color(struct tpg_data *tpg, int k)
g = tpg_colors[col].g;
b = tpg_colors[col].b;
} else if (tpg->pattern == TPG_PAT_NOISE) {
-   r = g = b = prandom_u32_max(256);
+   r = g = b = get_random_u8();
} else if (k == TPG_COLOR_RANDOM) {
r = g = b = tpg->qual_offset + prandom_u32_max(196);
} else if (k >= TPG_COLOR_RAMP) {
diff --git a/drivers/media/test-drivers/vivid/vivid-radio-rx.c 
b/drivers/media/test-drivers/vivid/vivid-radio-rx.c
index 232cab508f48..8bd09589fb15 100644
--- a/drivers/media/test-drivers/vivid/vivid-radio-rx.c
+++ b/drivers/media/test-drivers/vivid/vivid-radio-rx.c
@@ -104,8 +104,8 @@ ssize_t vivid_radio_rx_read(struct file *file, char __user 
*buf,
break;
case 2:
rds.block |= V4L2_RDS_BLOCK_ERROR;
-   rds.lsb = prandom_u32_max(256);
-   rds.msb = prandom_u32_max(256);
+   rds.lsb = get_random_u8();
+   rds.msb = get_random_u8();
break;
case 3: /* Skip block altogether */
if (i)
diff --git a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c 
b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
index ddfe9208529a..ac452a0111a9 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
+++ b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
@@ -1467,7 +1467,7 @@ static void make_established(struct sock *sk, u32 
snd_isn, unsigned int opt)
tp->write_seq = snd_isn;
tp->snd_nxt = snd_isn;
tp->snd_una = snd_isn;
-   inet_sk(sk)->inet_id = prandom_u32();
+   inet_sk(sk)

Re: [PATCH v1 0/5] treewide cleanup of random integer usage

On Wed, Oct 05, 2022 at 11:48:39PM +0200, Jason A. Donenfeld wrote:
> Hi folks,
> 
> This is a five part treewide cleanup of random integer handling. The
> rules for random integers are:
> 
> - If you want a secure or an insecure random u64, use get_random_u64().
> - If you want a secure or an insecure random u32, use get_random_u32().
>   * The old function prandom_u32() has been deprecated for a while now
> and is just a wrapper around get_random_u32().
> - If you want a secure or an insecure random u16, use get_random_u16().
> - If you want a secure or an insecure random u8, use get_random_u8().
> - If you want secure or insecure random bytes, use get_random_bytes().
>   * The old function prandom_bytes() has been deprecated for a while now
> and has long been a wrapper around get_random_bytes().
> - If you want a non-uniform random u32, u16, or u8 bounded by a certain
>   open interval maximum, use prandom_u32_max().
>   * I say "non-uniform", because it doesn't do any rejection sampling or
> divisions. Hence, it stays within the prandom_* namespace.
> 
> These rules ought to be applied uniformly, so that we can clean up the
> deprecated functions, and earn the benefits of using the modern
> functions. In particular, in addition to the boring substitutions, this
> patchset accomplishes a few nice effects:
> 
> - By using prandom_u32_max() with an upper-bound that the compiler can
>   prove at compile-time is ≤65536 or ≤256, internally get_random_u16()
>   or get_random_u8() is used, which wastes fewer batched random bytes,
>   and hence has higher throughput.
> 
> - By using prandom_u32_max() instead of %, when the upper-bound is not a
>   constant, division is still avoided, because prandom_u32_max() uses
>   a faster multiplication-based trick instead.
> 
> - By using get_random_u16() or get_random_u8() in cases where the return
>   value is intended to indeed be a u16 or a u8, we waste fewer batched
>   random bytes, and hence have higher throughput.
> 
> So, based on those rules and benefits from following them, this patchset
> breaks down into the following five steps:
> 
> 1) Replace `prandom_u32() % max` and variants thereof with
>prandom_u32_max(max).
> 
> 2) Replace `(type)get_random_u32()` and variants thereof with
>get_random_u16() or get_random_u8(). I took the pains to actually
>look and see what every lvalue type was across the entire tree.
> 
> 3) Replace remaining deprecated uses of prandom_u32() with
>get_random_u32(). 
> 
> 4) Replace remaining deprecated uses of prandom_bytes() with
>get_random_bytes().
> 
> 5) Remove the deprecated and now-unused prandom_u32() and
>prandom_bytes() inline wrapper functions.
> 
> I was thinking of taking this through my random.git tree (on which this
> series is currently based) and submitting it near the end of the merge
> window, or waiting for the very end of the 6.1 cycle when there will be
> the fewest new patches brewing. If somebody with some treewide-cleanup
> experience might share some wisdom about what the best timing usually
> winds up being, I'm all ears.

It'd be nice to capture some (all?) of the above somewhere. Perhaps just
a massive comment in the header?

> I've CC'd get_maintainers.pl, which is a pretty big list. Probably some
> portion of those are going to bounce, too, and everytime you reply to
> this thread, you'll have to deal with a bunch of bounces coming
> immediately after. And a recipient list this big will probably dock my
> email domain's spam reputation, at least temporarily. Sigh. I think
> that's just how it goes with treewide cleanups though. Again, let me
> know if I'm doing it wrong.

I usually stick to just mailing lists and subsystem maintainers.

If any of the subsystems ask you to break this up (I hope not), I've got
this[1], which does a reasonable job of splitting a commit up into
separate commits for each matching subsystem.

Showing that a treewide change can be reproduced mechanically helps with
keeping it together as one bit treewide patch, too, I've found. :)

Thank you for the cleanup! The "u8 rnd = get_random_u32()" in the tree
has bothered me for a lng time.

-Kees

-- 
Kees Cook

Re: [PATCH v1 1/5] treewide: use prandom_u32_max() when possible

2022-10-05 Thread KP Singh

On Wed, Oct 5, 2022 at 9:16 PM Kees Cook  wrote:
>
> On Wed, Oct 05, 2022 at 11:48:40PM +0200, Jason A. Donenfeld wrote:
> > Rather than incurring a division or requesting too many random bytes for
> > the given range, use the prandom_u32_max() function, which only takes
> > the minimum required bytes from the RNG and avoids divisions.
>
> Yes please!
>
> Since this is a treewide patch, it's helpful for (me at least) doing
> reviews to detail the mechanism of the transformation.
>
> e.g. I imagine this could be done with something like Coccinelle and
>
> @no_modulo@
> expression E;
> @@
>
> -   (prandom_u32() % (E))
> +   prandom_u32_max(E)
>
> > diff --git a/drivers/mtd/ubi/debug.h b/drivers/mtd/ubi/debug.h
> > index 118248a5d7d4..4236c799a47c 100644
> > --- a/drivers/mtd/ubi/debug.h
> > +++ b/drivers/mtd/ubi/debug.h
> > @@ -73,7 +73,7 @@ static inline int ubi_dbg_is_bgt_disabled(const struct 
> > ubi_device *ubi)
> >  static inline int ubi_dbg_is_bitflip(const struct ubi_device *ubi)
> >  {
> >   if (ubi->dbg.emulate_bitflips)
> > - return !(prandom_u32() % 200);
> > + return !(prandom_u32_max(200));
> >   return 0;
> >  }
> >
>
> Because some looks automated (why the parens?)
>
> > @@ -393,14 +387,11 @@ static struct test_driver {
> >
> >  static void shuffle_array(int *arr, int n)
> >  {
> > - unsigned int rnd;
> >   int i, j;
> >
> >   for (i = n - 1; i > 0; i--)  {
> > - rnd = prandom_u32();
> > -
> >   /* Cut the range. */
> > - j = rnd % i;
> > + j = prandom_u32_max(i);
> >
> >   /* Swap indexes. */
> >   swap(arr[i], arr[j]);
>
> And some by hand. :)
>
> Reviewed-by: Kees Cook 

Thanks!

Reviewed-by: KP Singh 


>
> --
> Kees Cook

[PATCH v1 1/5] treewide: use prandom_u32_max() when possible

Rather than incurring a division or requesting too many random bytes for
the given range, use the prandom_u32_max() function, which only takes
the minimum required bytes from the RNG and avoids divisions.

Signed-off-by: Jason A. Donenfeld 
---
 arch/x86/mm/pat/cpa-test.c|  4 +-
 crypto/testmgr.c  | 86 +--
 drivers/block/drbd/drbd_receiver.c|  4 +-
 drivers/infiniband/core/cma.c |  2 +-
 drivers/infiniband/hw/cxgb4/id_table.c|  4 +-
 drivers/infiniband/hw/hns/hns_roce_ah.c   |  5 +-
 drivers/infiniband/ulp/rtrs/rtrs-clt.c|  3 +-
 drivers/mmc/core/core.c   |  4 +-
 drivers/mmc/host/dw_mmc.c |  2 +-
 drivers/mtd/nand/raw/nandsim.c|  4 +-
 drivers/mtd/tests/mtd_nandecctest.c   | 10 +--
 drivers/mtd/tests/stresstest.c| 17 +---
 drivers/mtd/ubi/debug.c   |  2 +-
 drivers/mtd/ubi/debug.h   |  6 +-
 drivers/net/ethernet/broadcom/cnic.c  |  3 +-
 .../chelsio/inline_crypto/chtls/chtls_io.c|  4 +-
 drivers/net/hamradio/baycom_epp.c |  2 +-
 drivers/net/hamradio/hdlcdrv.c|  2 +-
 drivers/net/hamradio/yam.c|  2 +-
 drivers/net/phy/at803x.c  |  2 +-
 .../broadcom/brcm80211/brcmfmac/p2p.c |  2 +-
 .../net/wireless/intel/iwlwifi/mvm/mac-ctxt.c |  2 +-
 drivers/scsi/fcoe/fcoe_ctlr.c |  4 +-
 drivers/scsi/qedi/qedi_main.c |  2 +-
 fs/ceph/inode.c   |  2 +-
 fs/ceph/mdsmap.c  |  2 +-
 fs/ext4/super.c   |  7 +-
 fs/f2fs/gc.c  |  2 +-
 fs/f2fs/segment.c |  8 +-
 fs/ubifs/debug.c  |  8 +-
 fs/ubifs/lpt_commit.c | 14 +--
 fs/ubifs/tnc_commit.c |  2 +-
 fs/xfs/libxfs/xfs_alloc.c |  2 +-
 fs/xfs/libxfs/xfs_ialloc.c|  2 +-
 fs/xfs/xfs_error.c|  2 +-
 kernel/time/clocksource.c |  2 +-
 lib/fault-inject.c|  2 +-
 lib/find_bit_benchmark.c  |  4 +-
 lib/reed_solomon/test_rslib.c |  6 +-
 lib/sbitmap.c |  4 +-
 lib/test_list_sort.c  |  2 +-
 lib/test_vmalloc.c| 17 +---
 net/ceph/mon_client.c |  2 +-
 net/ceph/osd_client.c |  2 +-
 net/core/neighbour.c  |  2 +-
 net/core/pktgen.c | 43 +-
 net/core/stream.c |  2 +-
 net/ipv4/igmp.c   |  6 +-
 net/ipv4/inet_connection_sock.c   |  2 +-
 net/ipv4/inet_hashtables.c|  2 +-
 net/ipv6/addrconf.c   |  8 +-
 net/ipv6/mcast.c  | 10 +--
 net/netfilter/ipvs/ip_vs_twos.c   |  4 +-
 net/packet/af_packet.c|  2 +-
 net/sched/act_gact.c  |  2 +-
 net/sched/act_sample.c|  2 +-
 net/sched/sch_netem.c |  4 +-
 net/sctp/socket.c |  2 +-
 net/sunrpc/cache.c|  2 +-
 net/sunrpc/xprtsock.c |  2 +-
 net/tipc/socket.c |  2 +-
 net/xfrm/xfrm_state.c |  2 +-
 62 files changed, 173 insertions(+), 196 deletions(-)

diff --git a/arch/x86/mm/pat/cpa-test.c b/arch/x86/mm/pat/cpa-test.c
index 0612a73638a8..423b21e80929 100644
--- a/arch/x86/mm/pat/cpa-test.c
+++ b/arch/x86/mm/pat/cpa-test.c
@@ -136,10 +136,10 @@ static int pageattr_test(void)
failed += print_split(&sa);
 
for (i = 0; i < NTEST; i++) {
-   unsigned long pfn = prandom_u32() % max_pfn_mapped;
+   unsigned long pfn = prandom_u32_max(max_pfn_mapped);
 
addr[i] = (unsigned long)__va(pfn << PAGE_SHIFT);
-   len[i] = prandom_u32() % NPAGES;
+   len[i] = prandom_u32_max(NPAGES);
len[i] = min_t(unsigned long, len[i], max_pfn_mapped - pfn - 1);
 
if (len[i] == 0)
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 5349ffee6bbd..be45217acde4 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -855,9 +855,9 @@ static int prepare_keybuf(const u8 *key, unsigned int ksize,
 /* Generate a random length in range [0, max_len], but prefer smaller values */
 static unsigned int generate_random_length(unsigned int max_len)
 {
-   unsigned int len = prandom_u32() % (max_len + 1);
+   unsigned int len = prandom_u32_max(max_len + 1);
 
-   switch (prandom_u32() % 4) {
+   swit

[PATCH v3 2/2] drm/bridge: add it6505 driver to read data-lanes and max-pixel-clock-mhz from dt

2022-10-05 Thread allen

From: allen chen 

Add driver to read data-lanes and max-pixel-clock-mhz from dt property to
restrict output bandwidth.

Signed-off-by: Allen chen 
Signed-off-by: Pin-yen Lin 
---
 drivers/gpu/drm/bridge/ite-it6505.c | 36 ++---
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ite-it6505.c 
b/drivers/gpu/drm/bridge/ite-it6505.c
index 2767b70fa2cb..eca9a2c296a8 100644
--- a/drivers/gpu/drm/bridge/ite-it6505.c
+++ b/drivers/gpu/drm/bridge/ite-it6505.c
@@ -436,6 +436,8 @@ struct it6505 {
bool powered;
bool hpd_state;
u32 afe_setting;
+   u32 max_dpi_pixel_clock;
+   u32 max_lane_count;
enum hdcp_state hdcp_status;
struct delayed_work hdcp_work;
struct work_struct hdcp_wait_ksv_list;
@@ -1475,7 +1477,8 @@ static void it6505_parse_link_capabilities(struct it6505 
*it6505)
it6505->lane_count = link->num_lanes;
DRM_DEV_DEBUG_DRIVER(dev, "Sink support %d lanes training",
 it6505->lane_count);
-   it6505->lane_count = min_t(int, it6505->lane_count, MAX_LANE_COUNT);
+   it6505->lane_count = min_t(int, it6505->lane_count,
+  it6505->max_lane_count);
 
it6505->branch_device = drm_dp_is_branch(it6505->dpcd);
DRM_DEV_DEBUG_DRIVER(dev, "Sink %sbranch device",
@@ -2901,7 +2904,7 @@ it6505_bridge_mode_valid(struct drm_bridge *bridge,
if (mode->flags & DRM_MODE_FLAG_INTERLACE)
return MODE_NO_INTERLACE;
 
-   if (mode->clock > DPI_PIXEL_CLK_MAX)
+   if (mode->clock > it6505->max_dpi_pixel_clock)
return MODE_CLOCK_HIGH;
 
it6505->video_info.clock = mode->clock;
@@ -3066,6 +3069,8 @@ static void it6505_parse_dt(struct it6505 *it6505)
 {
struct device *dev = &it6505->client->dev;
u32 *afe_setting = &it6505->afe_setting;
+   u32 *max_lane_count = &it6505->max_lane_count;
+   u32 *max_dpi_pixel_clock = &it6505->max_dpi_pixel_clock;
 
it6505->lane_swap_disabled =
device_property_read_bool(dev, "no-laneswap");
@@ -3081,7 +3086,32 @@ static void it6505_parse_dt(struct it6505 *it6505)
} else {
*afe_setting = 0;
}
-   DRM_DEV_DEBUG_DRIVER(dev, "using afe_setting: %d", *afe_setting);
+
+   if (device_property_read_u32(dev, "ite,dp-output-data-lane-count",
+max_lane_count) == 0) {
+   if (*max_lane_count > 4 || *max_lane_count == 3) {
+   dev_err(dev, "max lane count error, use default");
+   *max_lane_count = MAX_LANE_COUNT;
+   }
+   } else {
+   *max_lane_count = MAX_LANE_COUNT;
+   }
+
+   if (device_property_read_u32(dev, "ite,dp-output-max-pixel-clock-mhz",
+max_dpi_pixel_clock) == 0) {
+   *max_dpi_pixel_clock *= 1000;
+   if (*max_dpi_pixel_clock > 297000) {
+   dev_err(dev, "max pixel clock error, use default");
+   *max_dpi_pixel_clock = DPI_PIXEL_CLK_MAX;
+   }
+   } else {
+   *max_dpi_pixel_clock = DPI_PIXEL_CLK_MAX;
+   }
+
+   DRM_DEV_DEBUG_DRIVER(dev, "using afe_setting: %u, max_lane_count: %u",
+it6505->afe_setting, it6505->max_lane_count);
+   DRM_DEV_DEBUG_DRIVER(dev, "using max_dpi_pixel_clock: %u kHz",
+it6505->max_dpi_pixel_clock);
 }
 
 static ssize_t receive_timing_debugfs_show(struct file *file, char __user *buf,
-- 
2.25.1

[PATCH v3 1/2] dt-bindings: it6505: add properties to restrict output bandwidth

2022-10-05 Thread allen

From: allen chen 

Add properties to restrict dp output data-lanes and clock.

Signed-off-by: Pin-Yen Lin 
Signed-off-by: Allen Chen 
---
 .../bindings/display/bridge/ite,it6505.yaml  | 12 
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml 
b/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml
index 833d11b2303a..f5482a614d05 100644
--- a/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml
@@ -52,6 +52,16 @@ properties:
 maxItems: 1
 description: extcon specifier for the Power Delivery
 
+  ite,dp-output-data-lane-count:
+description: restrict the dp output data-lanes with value of 1-4
+$ref: /schemas/types.yaml#/definitions/uint32
+enum: [ 1, 2, 4 ]
+
+  ite,dp-output-max-pixel-clock-mhz:
+description: restrict max pixel clock
+$ref: /schemas/types.yaml#/definitions/uint32
+default: 150
+
   port:
 $ref: /schemas/graph.yaml#/properties/port
 description: A port node pointing to DPI host port node
@@ -84,6 +94,8 @@ examples:
 pwr18-supply = <&it6505_pp18_reg>;
 reset-gpios = <&pio 179 1>;
 extcon = <&usbc_extcon>;
+ite,dp-output-data-lane-count = <2>;
+ite,dp-output-max-pixel-clock-mhz = <150>;
 
 port {
 it6505_in: endpoint {
-- 
2.25.1

[PATCH v3 0/2] * IT6505 driver read dt properties *

2022-10-05 Thread allen

From: allen chen 

This series let driver can read properties from dt to restrict dp output
bandwidth.

Changes in v3:
-Rename property name.

allen chen (2):
  dt-bindings: it6505: add properties to restrict output bandwidth
  drm/bridge: add it6505 driver to read data-lanes and
max-pixel-clock-mhz from dt

 .../bindings/display/bridge/ite,it6505.yaml   | 12 +++
 drivers/gpu/drm/bridge/ite-it6505.c   | 36 +--
 2 files changed, 45 insertions(+), 3 deletions(-)

-- 
2.25.1

Re: [PATCH v2] drm/ssd130x: Iterate over damage clips instead of using a merged rect

2022-10-05 Thread Javier Martinez Canillas

On 10/5/22 19:18, Thomas Zimmermann wrote:
> 
> 
> Am 30.09.22 um 17:29 schrieb Javier Martinez Canillas:
>> The drm_atomic_helper_damage_merged() helper merges all the damage clips
>> into one rectangle. If there are multiple damage clips that aren't close
>> to each other, the resulting rectangle could be quite big.
>>
>> Instead of using that function helper, iterate over all the damage clips
>> and update them one by one.
>>
>> Suggested-by: Jocelyn Falempe 
>> Signed-off-by: Javier Martinez Canillas 
> 
> Acked-by: Thomas Zimmermann 
> 

Pushed this to drm-misc (drm-misc-next) now. Thanks!

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: linux-next: build failure after merge of the drm tree

2022-10-05 Thread Stephen Rothwell

Hi all,

On Wed, 5 Oct 2022 12:45:31 -0400 Hamza Mahfooz  wrote:
>
> On 2022-10-05 11:30, Alex Deucher wrote:
> > @Mahfooz, Hamza
> > @Aurabindo Pillai can you get this fixed up?
> >   
> 
> Seems like this is a false positive for GCC versions pre-12, because I'm not 
> seeing this warning on GCC 12.2.
> However, we can fix this for older GCC versions with the following:
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dc_stream.h 
> b/drivers/gpu/drm/amd/display/dc/dc_stream.h
> index 9e6025c98db9..67fede4bf248 100644
> --- a/drivers/gpu/drm/amd/display/dc/dc_stream.h
> +++ b/drivers/gpu/drm/amd/display/dc/dc_stream.h
> @@ -238,7 +238,7 @@ struct dc_stream_state {
> 
>   /* writeback */
>   unsigned int num_wb_info;
> - struct dc_writeback_info writeback_info[MAX_DWB_PIPES];
> + struct dc_writeback_info writeback_info[MAX_DWB_PIPES + 1];
>   const struct dc_transfer_func *func_shaper;
>   const struct dc_3dlut *lut3d_func;
>   /* Computed state bits */

This is now in Linus' tree :-(

I have applied the following hack for today:

From: Stephen Rothwell 
Date: Thu, 6 Oct 2022 09:14:26 +1100
Subject: [PATCH] fix up for drivers/gpu/drm/amd/display/dc/core/dc_stream.c

Signed-off-by: Stephen Rothwell 
---
 drivers/gpu/drm/amd/display/dc/core/dc_stream.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
index ae13887756bf..a5da787b7876 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
@@ -520,9 +520,9 @@ bool dc_stream_remove_writeback(struct dc *dc,
}
 
/* remove writeback info for disabled writeback pipes from stream */
-   for (i = 0, j = 0; i < stream->num_wb_info && j < MAX_DWB_PIPES; i++) {
+   for (i = 0, j = 0; i < stream->num_wb_info && i < MAX_DWB_PIPES; i++) {
if (stream->writeback_info[i].wb_enabled) {
-   if (i != j)
+   if ((j >= 0) && (j < i))
/* trim the array */
stream->writeback_info[j] = 
stream->writeback_info[i];
j++;
-- 
2.35.1

-- 
Cheers,
Stephen Rothwell


pgpQz7CGilWZb.pgp
Description: OpenPGP digital signature

Re: [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call

2022-10-05 Thread Alex Williamson

On Wed, 5 Oct 2022 14:17:17 -0600
Alex Williamson  wrote:

> On Thu, 29 Sep 2022 14:48:35 -0300
> Jason Gunthorpe  wrote:
> 
> > When converting to directly create the vfio_device the mdev driver has to
> > put a vfio_register_emulated_iommu_dev() in the probe() and a pairing
> > vfio_unregister_group_dev() in the remove.
> > 
> > This was missed for gvt, add it.
> > 
> > Cc: sta...@vger.kernel.org
> > Fixes: 978cf586ac35 ("drm/i915/gvt: convert to use 
> > vfio_register_emulated_iommu_dev")
> > Reported-by: Alex Williamson 
> > Signed-off-by: Jason Gunthorpe 
> > ---
> >  drivers/gpu/drm/i915/gvt/kvmgt.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > Should go through Alex's tree.
> > 
> > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c 
> > b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > index 41bba40feef8f4..9003145adb5a93 100644
> > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > @@ -1615,6 +1615,7 @@ static void intel_vgpu_remove(struct mdev_device 
> > *mdev)
> > if (WARN_ON_ONCE(vgpu->attached))
> > return;

Actually, what's the purpose of this  ?

We can't have a .remove callback that does nothing, this breaks
removing the device while it's in use.  Once we have the
vfio_unregister_group_dev() fix below, we'll block until the device is
unused, at which point vgpu->attached becomes false.  Unless I'm
missing something, I think we should also follow-up with a patch to
remove that bogus warn-on branch, right?  Thanks,

Alex

> >  
> > +   vfio_unregister_group_dev(&vgpu->vfio_device);
> > vfio_put_device(&vgpu->vfio_device);
> >  }
> >  
> > 
> > base-commit: c72e0034e6d4c36322d958b997d11d2627c6056c  
> 
> This is marked for stable, but I think the stable backport for
> existing kernels is actually:
> 
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c 
> b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index e3cd58946477..de89946c4817 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -1595,6 +1595,9 @@ static void intel_vgpu_remove(struct mdev_device *mdev)
>  
>   if (WARN_ON_ONCE(vgpu->attached))
>   return;
> +
> + vfio_unregister_group_dev(&vgpu->vfio_device);
> + vfio_uninit_group_dev(&vgpu->vfio_device);
>   intel_gvt_destroy_vgpu(vgpu);
>  }

Re: [PATCH v2 4/7] drm/msm/dsi: Reuse earlier computed dsc->slice_chunk_size





On 10/5/2022 11:16 AM, Marijn Suijten wrote:

dsi_populate_dsc_params() is called prior to dsi_update_dsc_timing() and
already computes a value for slice_chunk_size, whose value doesn't need
to be recomputed and re-set here.

Fixes: 08802f515c3c ("drm/msm/dsi: Add support for DSC configuration")
Signed-off-by: Marijn Suijten 

Reviewed-by: Abhinav Kumar 

---
  drivers/gpu/drm/msm/dsi/dsi_host.c | 9 ++---
  1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 48c966375ffa..f42794cdd4c1 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -845,7 +845,6 @@ static void dsi_update_dsc_timing(struct msm_dsi_host 
*msm_host, bool is_cmd_mod
u32 reg, reg_ctrl, reg_ctrl2;
u32 slice_per_intf, total_bytes_per_intf;
u32 pkt_per_line;
-   u32 bytes_in_slice;
u32 eol_byte_num;
  
  	/* first calculate dsc parameters and then program

@@ -860,11 +859,7 @@ static void dsi_update_dsc_timing(struct msm_dsi_host 
*msm_host, bool is_cmd_mod
if (slice_per_intf > dsc->slice_count)
dsc->slice_count = 1;
  
-	bytes_in_slice = DIV_ROUND_UP(dsc->slice_width * dsc->bits_per_pixel, 8);

-
-   dsc->slice_chunk_size = bytes_in_slice;
-
-   total_bytes_per_intf = bytes_in_slice * slice_per_intf;
+   total_bytes_per_intf = dsc->slice_chunk_size * slice_per_intf;
  
  	eol_byte_num = total_bytes_per_intf % 3;

pkt_per_line = slice_per_intf / dsc->slice_count;
@@ -890,7 +885,7 @@ static void dsi_update_dsc_timing(struct msm_dsi_host 
*msm_host, bool is_cmd_mod
reg_ctrl |= reg;
  
  		reg_ctrl2 &= ~DSI_COMMAND_COMPRESSION_MODE_CTRL2_STREAM0_SLICE_WIDTH__MASK;

-   reg_ctrl2 |= 
DSI_COMMAND_COMPRESSION_MODE_CTRL2_STREAM0_SLICE_WIDTH(bytes_in_slice);
+   reg_ctrl2 |= 
DSI_COMMAND_COMPRESSION_MODE_CTRL2_STREAM0_SLICE_WIDTH(dsc->slice_chunk_size);
  
  		dsi_write(msm_host, REG_DSI_COMMAND_COMPRESSION_MODE_CTRL, reg_ctrl);

dsi_write(msm_host, REG_DSI_COMMAND_COMPRESSION_MODE_CTRL2, 
reg_ctrl2);

Re: [PATCH v2 3/7] drm/msm/dsi: Use DIV_ROUND_UP instead of conditional increment on modulo





On 10/5/2022 11:16 AM, Marijn Suijten wrote:

This exact same math is used to compute bytes_in_slice above in
dsi_update_dsc_timing(), also used to fill slice_chunk_size.

Fixes: b9080324d6ca ("drm/msm/dsi: add support for dsc data")
Signed-off-by: Marijn Suijten 

Reviewed-by: Abhinav Kumar 

---
  drivers/gpu/drm/msm/dsi/dsi_host.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index c746ed5d61f9..48c966375ffa 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -1829,9 +1829,7 @@ static int dsi_populate_dsc_params(struct drm_dsc_config 
*dsc)
 * params are calculated
 */
groups_per_line = DIV_ROUND_UP(dsc->slice_width, 3);
-   dsc->slice_chunk_size = dsc->slice_width * dsc->bits_per_pixel / 8;
-   if ((dsc->slice_width * dsc->bits_per_pixel) % 8)
-   dsc->slice_chunk_size++;
+   dsc->slice_chunk_size = DIV_ROUND_UP(dsc->slice_width * 
dsc->bits_per_pixel, 8);
  
  	/* rbs-min */

min_rate_buffer_size =  dsc->rc_model_size - dsc->initial_offset +

Re: [PATCH v2 5/7] drm/msm/dsi: Account for DSC's bits_per_pixel having 4 fractional bits

On 2022-10-06 00:11:06, Dmitry Baryshkov wrote:
> On 06/10/2022 00:08, Marijn Suijten wrote:
> > [..]
> > Aside fixing that to assign these values (through the proper constants)
> > to dsc->mux_word_size, is it worth looking for the right parameters for
> > other bpc or return -EINVAL if bpc isn't 8, to uphold this comment until
> > support for other bpc is validated?
> 
> I'd say, return -EINVAL or -EOPNOTSUPP for now, let's fix that later. 
> It's definitely a separate change. Let's wait for a device with such 
> panel to be able to test it.

According to [1] these four fields specifically are different for other
BPC; I can add a -EOPNOTSUPP and DRM_DEV_ERROR requesting a test, or
insert the values; there's only 8BPC and 10BPC, no 12BPC.

Aside that we need a different initial_offset = 5632 for other bpp/bpc
pairs.

[1]: 
https://git.codelinaro.org/clo/la/platform/vendor/opensource/display-drivers/-/blob/DISPLAY.LA.2.0.r1-08000-WAIPIO.0/msm/sde_dsc_helper.c#L110-139

- Marijn

Re: [PATCH v2 1/7] drm/msm/dsi: Remove useless math in DSC calculations





On 10/5/2022 11:16 AM, Marijn Suijten wrote:

Multiplying a value by 2 and adding 1 to it always results in a value
that is uneven, and that 1 gets truncated immediately when performing
integer division by 2 again.  There is no "rounding" possible here.

After that target_bpp_x16 is used to store a multiplication of
bits_per_pixel by 16 which is only ever read to immediately be divided
by 16 again, and is elided in much the same way.

Fixes: b9080324d6ca ("drm/msm/dsi: add support for dsc data")
Signed-off-by: Marijn Suijten 

Reviewed-by: Abhinav Kumar 

---
  drivers/gpu/drm/msm/dsi/dsi_host.c | 10 +-
  1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 8e4bc586c262..70077d1f0f21 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -1784,7 +1784,6 @@ static int dsi_populate_dsc_params(struct drm_dsc_config 
*dsc)
int hrd_delay;
int pre_num_extra_mux_bits, num_extra_mux_bits;
int slice_bits;
-   int target_bpp_x16;
int data;
int final_value, final_scale;
int i;
@@ -1864,14 +1863,7 @@ static int dsi_populate_dsc_params(struct drm_dsc_config 
*dsc)
data = 2048 * (dsc->rc_model_size - dsc->initial_offset + 
num_extra_mux_bits);
dsc->slice_bpg_offset = DIV_ROUND_UP(data, groups_total);
  
-	/* bpp * 16 + 0.5 */

-   data = dsc->bits_per_pixel * 16;
-   data *= 2;
-   data++;
-   data /= 2;
-   target_bpp_x16 = data;
-
-   data = (dsc->initial_xmit_delay * target_bpp_x16) / 16;
+   data = dsc->initial_xmit_delay * dsc->bits_per_pixel;
final_value =  dsc->rc_model_size - data + num_extra_mux_bits;
dsc->final_offset = final_value;

Re: [PATCH v2 5/7] drm/msm/dsi: Account for DSC's bits_per_pixel having 4 fractional bits

2022-10-05 Thread Dmitry Baryshkov


On 06/10/2022 00:08, Marijn Suijten wrote:

On 2022-10-05 22:58:48, Marijn Suijten wrote:

On 2022-10-05 22:31:43, Dmitry Baryshkov wrote:

[..]
In fact, could you please take a look if we can switch to using this
function and drop our code?


[..]

Do you want me to do this in a v3, before applying this fractional-bits
fix?  [..]


One thing to note:

/* bpc 8 */
dsc->flatness_min_qp = 3;
dsc->flatness_max_qp = 12;
dsc->rc_quant_incr_limit0 = 11;
dsc->rc_quant_incr_limit1 = 11;
dsc->mux_word_size = DSC_MUX_WORD_SIZE_8_10_BPC;

Here a bunch of properties are hardcoded, seemingly for bpc = 8.
mux_word_size is only ever read in drm_dsc_compute_rc_parameters() so
only becomes relevant _after_ the migration, and is currently dealt with
correctly by:

mux_words_size = 48;/* bpc == 8/10 */
if (dsc->bits_per_component == 12)
mux_words_size = 64;

Aside fixing that to assign these values (through the proper constants)
to dsc->mux_word_size, is it worth looking for the right parameters for
other bpc or return -EINVAL if bpc isn't 8, to uphold this comment until
support for other bpc is validated?


I'd say, return -EINVAL or -EOPNOTSUPP for now, let's fix that later. 
It's definitely a separate change. Let's wait for a device with such 
panel to be able to test it.


--
With best wishes
Dmitry

Re: [PATCH v2 5/7] drm/msm/dsi: Account for DSC's bits_per_pixel having 4 fractional bits

On 2022-10-05 22:58:48, Marijn Suijten wrote:
> On 2022-10-05 22:31:43, Dmitry Baryshkov wrote:
> > [..]
> > In fact, could you please take a look if we can switch to using this
> > function and drop our code?
>
> [..]
>
> Do you want me to do this in a v3, before applying this fractional-bits
> fix?  [..]

One thing to note:

/* bpc 8 */
dsc->flatness_min_qp = 3;
dsc->flatness_max_qp = 12;
dsc->rc_quant_incr_limit0 = 11;
dsc->rc_quant_incr_limit1 = 11;
dsc->mux_word_size = DSC_MUX_WORD_SIZE_8_10_BPC;

Here a bunch of properties are hardcoded, seemingly for bpc = 8.
mux_word_size is only ever read in drm_dsc_compute_rc_parameters() so
only becomes relevant _after_ the migration, and is currently dealt with
correctly by:

mux_words_size = 48;/* bpc == 8/10 */
if (dsc->bits_per_component == 12)
mux_words_size = 64;

Aside fixing that to assign these values (through the proper constants)
to dsc->mux_word_size, is it worth looking for the right parameters for
other bpc or return -EINVAL if bpc isn't 8, to uphold this comment until
support for other bpc is validated?

- Marijn

Re: [PATCH v2 5/7] drm/msm/dsi: Account for DSC's bits_per_pixel having 4 fractional bits

2022-10-05 Thread Dmitry Baryshkov


On 05/10/2022 23:58, Marijn Suijten wrote:

On 2022-10-05 22:31:43, Dmitry Baryshkov wrote:

On Wed, 5 Oct 2022 at 21:17, Marijn Suijten
 wrote:


drm_dsc_config's bits_per_pixel field holds a fractional value with 4
bits, which all panel drivers should adhere to for
drm_dsc_pps_payload_pack() to generate a valid payload.  All code in the
DSI driver here seems to assume that this field doesn't contain any
fractional bits, hence resulting in the wrong values being computed.
Since none of the calculations leave any room for fractional bits or
seem to indicate any possible area of support, disallow such values
altogether.

Fixes: b9080324d6ca ("drm/msm/dsi: add support for dsc data")
Signed-off-by: Marijn Suijten 
---
  drivers/gpu/drm/msm/dsi/dsi_host.c | 25 +
  1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index f42794cdd4c1..4717d49d76be 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -33,7 +33,7 @@

  #define DSI_RESET_TOGGLE_DELAY_MS 20

-static int dsi_populate_dsc_params(struct drm_dsc_config *dsc);
+static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct 
drm_dsc_config *dsc);

  static int dsi_get_version(const void __iomem *base, u32 *major, u32 *minor)
  {
@@ -908,6 +908,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, 
bool is_bonded_dsi)
 u32 va_end = va_start + mode->vdisplay;
 u32 hdisplay = mode->hdisplay;
 u32 wc;
+   int ret;

 DBG("");

@@ -943,7 +944,9 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, 
bool is_bonded_dsi)
 /* we do the calculations for dsc parameters here so that
  * panel can use these parameters
  */
-   dsi_populate_dsc_params(dsc);
+   ret = dsi_populate_dsc_params(msm_host, dsc);
+   if (ret)
+   return;

 /* Divide the display by 3 but keep back/font porch and
  * pulse width same
@@ -1769,7 +1772,7 @@ static char bpg_offset[DSC_NUM_BUF_RANGES] = {
 2, 0, 0, -2, -4, -6, -8, -8, -8, -10, -10, -12, -12, -12, -12
  };

-static int dsi_populate_dsc_params(struct drm_dsc_config *dsc)
+static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct 
drm_dsc_config *dsc)
  {
 int mux_words_size;
 int groups_per_line, groups_total;
@@ -1780,6 +1783,12 @@ static int dsi_populate_dsc_params(struct drm_dsc_config 
*dsc)
 int data;
 int final_value, final_scale;
 int i;
+   u16 bpp = dsc->bits_per_pixel >> 4;
+
+   if (dsc->bits_per_pixel & 0xf) {
+   DRM_DEV_ERROR(&msm_host->pdev->dev, "DSI does not support fractional 
bits_per_pixel\n");
+   return -EINVAL;
+   }

 dsc->rc_model_size = 8192;
 dsc->first_line_bpg_offset = 12;
@@ -1801,7 +1810,7 @@ static int dsi_populate_dsc_params(struct drm_dsc_config 
*dsc)
 }

 dsc->initial_offset = 6144; /* Not bpp 12 */
-   if (dsc->bits_per_pixel != 8)
+   if (bpp != 8)
 dsc->initial_offset = 2048; /* bpp = 12 */

 mux_words_size = 48;/* bpc == 8/10 */
@@ -1824,14 +1833,14 @@ static int dsi_populate_dsc_params(struct 
drm_dsc_config *dsc)
  * params are calculated
  */
 groups_per_line = DIV_ROUND_UP(dsc->slice_width, 3);
-   dsc->slice_chunk_size = DIV_ROUND_UP(dsc->slice_width * 
dsc->bits_per_pixel, 8);
+   dsc->slice_chunk_size = DIV_ROUND_UP(dsc->slice_width * bpp, 8);


I'd still prefer if we can get closer to drm_dsc_compute_rc_parameters().
The mentioned function has the following code:

vdsc_cfg->slice_chunk_size = DIV_ROUND_UP(vdsc_cfg->slice_width *

vdsc_cfg->bits_per_pixel,
   (8 * 16));


Fwiw this discussion happened in dsi_update_dsc_timing() where a similar
calculation was the sole user of bits_per_pixel.  This function,
dsi_populate_dsc_params(), has more uses of bits_per_pixel so it made
more sense to compute and document this "discrepancy" up front.
drm_dsc_compute_rc_parameters() doesn't document where this "16" comes
from, unfortunately (requiring knowledge of the contents of the struct).


In fact, could you please take a look if we can switch to using this
function and drop our code?


There's alread a:

/* FIXME: need to call drm_dsc_compute_rc_parameters() so that rest of
 * params are calculated
 */

And it was trivial to replace everything below that comment with this
function call; I have not checked the math in detail but it assigns
_every_ field that was also assigned here, and the resulting values
provide an identical DCS PPS (which I happened to be printing to compare
to downstream, leading to find all the issues solved in this series) and
working phone sc

Re: [PATCH v2 5/7] drm/msm/dsi: Account for DSC's bits_per_pixel having 4 fractional bits

On 2022-10-05 22:31:43, Dmitry Baryshkov wrote:
> On Wed, 5 Oct 2022 at 21:17, Marijn Suijten
>  wrote:
> >
> > drm_dsc_config's bits_per_pixel field holds a fractional value with 4
> > bits, which all panel drivers should adhere to for
> > drm_dsc_pps_payload_pack() to generate a valid payload.  All code in the
> > DSI driver here seems to assume that this field doesn't contain any
> > fractional bits, hence resulting in the wrong values being computed.
> > Since none of the calculations leave any room for fractional bits or
> > seem to indicate any possible area of support, disallow such values
> > altogether.
> >
> > Fixes: b9080324d6ca ("drm/msm/dsi: add support for dsc data")
> > Signed-off-by: Marijn Suijten 
> > ---
> >  drivers/gpu/drm/msm/dsi/dsi_host.c | 25 +
> >  1 file changed, 17 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
> > b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > index f42794cdd4c1..4717d49d76be 100644
> > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > @@ -33,7 +33,7 @@
> >
> >  #define DSI_RESET_TOGGLE_DELAY_MS 20
> >
> > -static int dsi_populate_dsc_params(struct drm_dsc_config *dsc);
> > +static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct 
> > drm_dsc_config *dsc);
> >
> >  static int dsi_get_version(const void __iomem *base, u32 *major, u32 
> > *minor)
> >  {
> > @@ -908,6 +908,7 @@ static void dsi_timing_setup(struct msm_dsi_host 
> > *msm_host, bool is_bonded_dsi)
> > u32 va_end = va_start + mode->vdisplay;
> > u32 hdisplay = mode->hdisplay;
> > u32 wc;
> > +   int ret;
> >
> > DBG("");
> >
> > @@ -943,7 +944,9 @@ static void dsi_timing_setup(struct msm_dsi_host 
> > *msm_host, bool is_bonded_dsi)
> > /* we do the calculations for dsc parameters here so that
> >  * panel can use these parameters
> >  */
> > -   dsi_populate_dsc_params(dsc);
> > +   ret = dsi_populate_dsc_params(msm_host, dsc);
> > +   if (ret)
> > +   return;
> >
> > /* Divide the display by 3 but keep back/font porch and
> >  * pulse width same
> > @@ -1769,7 +1772,7 @@ static char bpg_offset[DSC_NUM_BUF_RANGES] = {
> > 2, 0, 0, -2, -4, -6, -8, -8, -8, -10, -10, -12, -12, -12, -12
> >  };
> >
> > -static int dsi_populate_dsc_params(struct drm_dsc_config *dsc)
> > +static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct 
> > drm_dsc_config *dsc)
> >  {
> > int mux_words_size;
> > int groups_per_line, groups_total;
> > @@ -1780,6 +1783,12 @@ static int dsi_populate_dsc_params(struct 
> > drm_dsc_config *dsc)
> > int data;
> > int final_value, final_scale;
> > int i;
> > +   u16 bpp = dsc->bits_per_pixel >> 4;
> > +
> > +   if (dsc->bits_per_pixel & 0xf) {
> > +   DRM_DEV_ERROR(&msm_host->pdev->dev, "DSI does not support 
> > fractional bits_per_pixel\n");
> > +   return -EINVAL;
> > +   }
> >
> > dsc->rc_model_size = 8192;
> > dsc->first_line_bpg_offset = 12;
> > @@ -1801,7 +1810,7 @@ static int dsi_populate_dsc_params(struct 
> > drm_dsc_config *dsc)
> > }
> >
> > dsc->initial_offset = 6144; /* Not bpp 12 */
> > -   if (dsc->bits_per_pixel != 8)
> > +   if (bpp != 8)
> > dsc->initial_offset = 2048; /* bpp = 12 */
> >
> > mux_words_size = 48;/* bpc == 8/10 */
> > @@ -1824,14 +1833,14 @@ static int dsi_populate_dsc_params(struct 
> > drm_dsc_config *dsc)
> >  * params are calculated
> >  */
> > groups_per_line = DIV_ROUND_UP(dsc->slice_width, 3);
> > -   dsc->slice_chunk_size = DIV_ROUND_UP(dsc->slice_width * 
> > dsc->bits_per_pixel, 8);
> > +   dsc->slice_chunk_size = DIV_ROUND_UP(dsc->slice_width * bpp, 8);
> 
> I'd still prefer if we can get closer to drm_dsc_compute_rc_parameters().
> The mentioned function has the following code:
> 
> vdsc_cfg->slice_chunk_size = DIV_ROUND_UP(vdsc_cfg->slice_width *
> 
> vdsc_cfg->bits_per_pixel,
>   (8 * 16));

Fwiw this discussion happened in dsi_update_dsc_timing() where a similar
calculation was the sole user of bits_per_pixel.  This function,
dsi_populate_dsc_params(), has more uses of bits_per_pixel so it made
more sense to compute and document this "discrepancy" up front.
drm_dsc_compute_rc_parameters() doesn't document where this "16" comes
from, unfortunately (requiring knowledge of the contents of the struct).

> In fact, could you please take a look if we can switch to using this
> function and drop our code?

There's alread a:

/* FIXME: need to call drm_dsc_compute_rc_parameters() so that rest of
 * params are calculated
 */

And it was trivial to replace everything below that com

[linux-next:master] BUILD REGRESSION 67ae4f7434cee86ee318d46fb10b8a9840ad2e81

2022-10-05 Thread kernel test robot

tree/branch: 
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
branch HEAD: 67ae4f7434cee86ee318d46fb10b8a9840ad2e81  Add linux-next specific 
files for 20221005

Error/Warning reports:

https://lore.kernel.org/linux-mm/202209060229.dvuyxjbv-...@intel.com
https://lore.kernel.org/llvm/202209220019.yr2vuxhg-...@intel.com
https://lore.kernel.org/llvm/202210041553.k9dc1joc-...@intel.com
https://lore.kernel.org/llvm/202210060148.uxbijocs-...@intel.com

Error/Warning: (recently discovered and may have been fixed)

ERROR: modpost: "devm_ioremap_resource" [drivers/dma/fsl-edma.ko] undefined!
ERROR: modpost: "devm_ioremap_resource" [drivers/dma/idma64.ko] undefined!
ERROR: modpost: "devm_ioremap_resource" [drivers/dma/qcom/hdma.ko] undefined!
ERROR: modpost: "devm_memremap" [drivers/misc/open-dice.ko] undefined!
ERROR: modpost: "devm_memunmap" [drivers/misc/open-dice.ko] undefined!
ERROR: modpost: "devm_platform_ioremap_resource" 
[drivers/char/xillybus/xillybus_of.ko] undefined!
ERROR: modpost: "ioremap" [drivers/net/ethernet/8390/pcnet_cs.ko] undefined!
ERROR: modpost: "ioremap" [drivers/tty/ipwireless/ipwireless.ko] undefined!
ERROR: modpost: "iounmap" [drivers/net/ethernet/8390/pcnet_cs.ko] undefined!
ERROR: modpost: "iounmap" [drivers/tty/ipwireless/ipwireless.ko] undefined!
arch/arm64/kernel/alternative.c:199:6: warning: no previous prototype for 
'apply_alternatives_vdso' [-Wmissing-prototypes]
arch/arm64/kernel/alternative.c:295:14: warning: no previous prototype for 
'alt_cb_patch_nops' [-Wmissing-prototypes]
arch/loongarch/kernel/traps.c:250 die() warn: variable dereferenced before 
check 'regs' (see line 244)
arch/loongarch/mm/init.c:166:24: warning: variable 'new' set but not used 
[-Wunused-but-set-variable]
drivers/gpu/drm/amd/amdgpu/../display/dc/virtual/virtual_link_hwss.c:40:6: 
warning: no previous prototype for 'virtual_disable_link_output' 
[-Wmissing-prototypes]
drivers/gpu/drm/bridge/ite-it6505.c:2712 it6505_extcon_work() warn: 
pm_runtime_get_sync() also returns 1 on success
drivers/platform/loongarch/loongson-laptop.c:377 
loongson_laptop_get_brightness() warn: impossible condition '(level < 0) => 
(0-255 < 0)'
drivers/vfio/pci/vfio_pci_core.c:775 vfio_pci_ioctl_get_region_info() warn: 
potential spectre issue 'pdev->resource' [w]
drivers/vfio/pci/vfio_pci_core.c:855 vfio_pci_ioctl_get_region_info() warn: 
potential spectre issue 'vdev->region' [r]
fs/ext4/super.c:1744:19: warning: 'deprecated_msg' defined but not used 
[-Wunused-const-variable=]
include/linux/compiler_types.h:357:45: error: call to 
'__compiletime_assert_417' declared with attribute error: FIELD_GET: mask is 
not constant
kernel/bpf/memalloc.c:500 bpf_mem_alloc_destroy() error: potentially 
dereferencing uninitialized 'c'.
net/mac80211/iface.c:251 ieee80211_can_powered_addr_change() warn: inconsistent 
returns '&local->mtx'.

Unverified Error/Warning (likely false positive, please contact us if 
interested):

ERROR: modpost: "__tsan_memcpy" [arch/s390/crypto/ghash_s390.ko] undefined!
ERROR: modpost: "__tsan_memcpy" [arch/s390/crypto/sha512_s390.ko] undefined!
ERROR: modpost: "__tsan_memcpy" [fs/binfmt_misc.ko] undefined!
ERROR: modpost: "__tsan_memcpy" [fs/pstore/ramoops.ko] undefined!
ERROR: modpost: "__tsan_memset" [arch/s390/crypto/ghash_s390.ko] undefined!
ERROR: modpost: "__tsan_memset" [arch/s390/crypto/sha512_s390.ko] undefined!
ERROR: modpost: "__tsan_memset" [fs/autofs/autofs4.ko] undefined!
ERROR: modpost: "__tsan_memset" [fs/binfmt_misc.ko] undefined!
ERROR: modpost: "__tsan_memset" [fs/cramfs/cramfs.ko] undefined!
ERROR: modpost: "__tsan_memset" [fs/pstore/ramoops.ko] undefined!
s390x-linux-ld: self.c:(.text+0xf6): undefined reference to `__tsan_memcpy'
thread_self.c:(.text+0xe4): undefined reference to `__tsan_memcpy'

Error/Warning ids grouped by kconfigs:

gcc_recent_errors
|-- alpha-allyesconfig
|   `-- 
drivers-gpu-drm-amd-amdgpu-..-display-dc-virtual-virtual_link_hwss.c:warning:no-previous-prototype-for-virtual_disable_link_output
|-- alpha-randconfig-s042-20221002
|   |-- 
drivers-thermal-broadcom-ns-thermal.c:sparse:sparse:incorrect-type-in-argument-(different-address-spaces)-expected-void-data-got-void-noderef-__iomem-assigned-pvtmon
|   |-- 
drivers-thermal-broadcom-ns-thermal.c:sparse:sparse:incorrect-type-in-initializer-(different-address-spaces)-expected-void-noderef-__iomem-pvtmon-got-void
|   |-- 
drivers-thermal-broadcom-ns-thermal.c:sparse:sparse:incorrect-type-in-initializer-(different-address-spaces)-expected-void-noderef-__iomem-pvtmon-got-void-devdata
|   |-- 
fs-ext4-fast_commit.c:sparse:sparse:incorrect-type-in-argu

Re: [git pull] drm for 6.1-rc1

2022-10-05 Thread Dave Airlie

On Thu, 6 Oct 2022 at 04:38, Linus Torvalds
 wrote:
>
> On Tue, Oct 4, 2022 at 8:42 PM Dave Airlie  wrote:
> >
> > This is very conflict heavy, mostly the correct answer is picking
> > the version from drm-next.
>
> Ugh, yes, that was a bit annoying.
>
> I get the same end result as you did, but I do wonder if the drm
> people should try to keep some kind of separate "fixes" branches for
> things that go both into the development tree and then get sent to me
> for fixes pulls?
>
> Hopefully this "lots of pointless noise" was a one-off, but it might
> be due to how you guys work..

In this case I think it was a late set of fixes backported for new AMD
hardware, that had to be redone to fit into the current kernel that
caused most of it. I haven't seen it this bad in a long while. We also
maintain a rerere tree ourselves to avoid continuously seeing it.

The problem is a lot of developers don't have the insight that the
maintainers do into the current state of the tree/pipeline.

Stuff goes into next because that is where the patch it fixes
originally went, and it goes through CI there. Then at some point
someone else realises the change needs to be in fixes and it gets
backported.

The volume of patches and company signoff processes doesn't make it
trivial to upfront decide what needs to go in -next or -fixes
unfortunately.

Dave.

Re: [PATCH v7 00/10] drm: bridge: Add Samsung MIPI DSIM bridge

2022-10-05 Thread Marek Szyprowski

Hi Jagan,

On 05.10.2022 17:12, Jagan Teki wrote:
> This series supports common bridge support for Samsung MIPI DSIM
> which is used in Exynos and i.MX8MM SoC's.
>
> The final bridge supports both the Exynos and i.MX8MM DSI devices.
>
> Changes for v7:
> * fix the drm bridge attach chain for exynos drm dsi driver
> * fix the hw_type checking logic
>
> Changes for v6:
> * handle previous bridge for exynos dsi while attaching bridge
>
> Changes for v5:
> * bridge changes to support multi-arch
> * updated and clear commit messages
> * add hw_type via plat data
> * removed unneeded quirk
> * rebased on linux-next
>
> Changes for v4:
> * include Inki Dae in MAINTAINERS
> * remove dsi_driver probe in exynos_drm_drv to support multi-arch build
> * update init handling to ensure host init done on first cmd transfer
>
> Changes for v3:
> * fix the mult-arch build
> * fix dsi host init
> * updated commit messages
>
> Changes for v2:
> * fix bridge handling
> * fix dsi host init
> * correct the commit messages
>
> Patch 0001:   Samsung DSIM bridge
>
> Patch 0002:   PHY optional
>
> Patch 0003:   OF-graph or Child node lookup
>
> Patch 0004:   DSI host initialization
>
> Patch 0005:   atomic check
>
> Patch 0006:   PMS_P offset via plat data
>
> Patch 0007:   atomic_get_input_bus_fmts
>
> Patch 0008:   input_bus_flags
>
> Patch 0009:   document fsl,imx8mm-mipi-dsim
>
> Patch 0010:   add i.MX8MM DSIM support
>
> Tested in Engicam i.Core MX8M Mini SoM.

This finally doesn't break Exynos DSI. :) Feel free to add:

Acked-by: Marek Szyprowski

Tested-by: Marek Szyprowski

The next step would be to merge Dave's patchset and remove the hacks 
added here and there. Otherwise we will end up adding even more hacks soon.

> Repo:
> https://gitlab.com/openedev/kernel/-/commits/imx8mm-dsi-v7
>
> Any inputs?
> Jagan.
>
> Jagan Teki (10):
>drm: bridge: Add Samsung DSIM bridge driver
>drm: bridge: samsung-dsim: Lookup OF-graph or Child node devices
>drm: bridge: samsung-dsim: Mark PHY as optional
>drm: bridge: samsung-dsim: Handle proper DSI host initialization
>drm: bridge: samsung-dsim: Add atomic_check
>drm: bridge: samsung-dsim: Add platform PLL_P (PMS_P) offset
>drm: bridge: samsung-dsim: Add atomic_get_input_bus_fmts
>drm: bridge: samsung-dsim: Add input_bus_flags
>dt-bindings: display: exynos: dsim: Add NXP i.MX8MM support
>drm: bridge: samsung-dsim: Add i.MX8MM support
>
>   .../bindings/display/exynos/exynos_dsim.txt   |1 +
>   MAINTAINERS   |9 +
>   drivers/gpu/drm/bridge/Kconfig|   12 +
>   drivers/gpu/drm/bridge/Makefile   |1 +
>   drivers/gpu/drm/bridge/samsung-dsim.c | 1856 +
>   drivers/gpu/drm/exynos/Kconfig|1 +
>   drivers/gpu/drm/exynos/exynos_drm_dsi.c   | 1766 +---
>   include/drm/bridge/samsung-dsim.h |  115 +
>   8 files changed, 2108 insertions(+), 1653 deletions(-)
>   create mode 100644 drivers/gpu/drm/bridge/samsung-dsim.c
>   create mode 100644 include/drm/bridge/samsung-dsim.h
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH] drm/i915/gvt: Add missing vfio_unregister_group_dev() call

2022-10-05 Thread Alex Williamson

On Thu, 29 Sep 2022 14:48:35 -0300
Jason Gunthorpe  wrote:

> When converting to directly create the vfio_device the mdev driver has to
> put a vfio_register_emulated_iommu_dev() in the probe() and a pairing
> vfio_unregister_group_dev() in the remove.
> 
> This was missed for gvt, add it.
> 
> Cc: sta...@vger.kernel.org
> Fixes: 978cf586ac35 ("drm/i915/gvt: convert to use 
> vfio_register_emulated_iommu_dev")
> Reported-by: Alex Williamson 
> Signed-off-by: Jason Gunthorpe 
> ---
>  drivers/gpu/drm/i915/gvt/kvmgt.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> Should go through Alex's tree.
> 
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c 
> b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 41bba40feef8f4..9003145adb5a93 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -1615,6 +1615,7 @@ static void intel_vgpu_remove(struct mdev_device *mdev)
>   if (WARN_ON_ONCE(vgpu->attached))
>   return;
>  
> + vfio_unregister_group_dev(&vgpu->vfio_device);
>   vfio_put_device(&vgpu->vfio_device);
>  }
>  
> 
> base-commit: c72e0034e6d4c36322d958b997d11d2627c6056c

This is marked for stable, but I think the stable backport for
existing kernels is actually:

diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index e3cd58946477..de89946c4817 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -1595,6 +1595,9 @@ static void intel_vgpu_remove(struct mdev_device *mdev)
 
if (WARN_ON_ONCE(vgpu->attached))
return;
+
+   vfio_unregister_group_dev(&vgpu->vfio_device);
+   vfio_uninit_group_dev(&vgpu->vfio_device);
intel_gvt_destroy_vgpu(vgpu);
 }

drm fb helpers hotplug/resize

2022-10-05 Thread Zack Rusin

Hi, Thomas.

Because you've been the one who's been working on drm_fb_helper.c the most the 
last
few years I wanted to pick your brain a bit.

I was porting vmwgfx to drm_fb_helper code which is largely trivial, just 
removing
all of vmwgfx_fb.c and replacing it with a call to drm_fbdev_generic_setup. But
drm_fb_helper.c code never deals with resizes which is a bit of a problem.

e.g. replacing the drm_sysfs_hotplug_event() call from
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c#L2255
with drm_kms_helper_hotplug_event will call drm_fbdev_client_hotplug and end up 
in 
drm_fb_helper_hotplug_event:
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_fb_helper.c#L2003

Now drm_fb_helper_hotplug_event does drm_client_modeset_probe but it never 
resizes
drm_fb_helper::buffer and drm_fb_helper::fb so they're both incorrectly sized. 

In general I don't see drm_fb_helper code ever being able to deal with resizes. 
In
particular because the fbdev's xres_virtual/yres_virtual are sized exactly to 
the
initial xres/yres. 

It's definitely a lot bigger issue on virtualized environments where at boot 
we'll
have some very conservative size (800x600) on vmwgfx which is then usually 
resized
to the size of the window. drm_fb_helper breaks pretty bad in that case because 
it
can't deal with those resizes at all. 

Is this scenario something that drm_fb_helper should be able to handle or is it 
not
worth pursuing it? I don't think there's a trivial way of handling it so my 
guess is
that it would make drm_fb_helper quite a bit more complicated.

z

Re: [PATCH] drm/amdgpu/dm/mst: Fix incorrect usage of drm_dp_add_payload_part2()

2022-10-05 Thread Lyude Paul

On Tue, 2022-10-04 at 16:46 -0400, Rodrigo Siqueira Jordao wrote:
> 
> On 2022-10-04 16:24, Lyude Paul wrote:
> > Yikes, it appears somehow I totally made a mistake here. We're currently
> > checking to see if drm_dp_add_payload_part2() returns a non-zero value to
> > indicate success. That's totally wrong though, as this function only
> > returns a zero value on success - not the other way around.
> > 
> > So, fix that.
> > 
> > Signed-off-by: Lyude Paul 
> > Issue: https://gitlab.freedesktop.org/drm/amd/-/issues/2171
> > Fixes: 4d07b0bc4034 ("drm/display/dp_mst: Move all payload info into the 
> > atomic state")
> > ---
> >   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c 
> > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
> > index b8077fcd4651..00598def5b39 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
> > @@ -297,7 +297,7 @@ bool dm_helpers_dp_mst_send_payload_allocation(
> > clr_flag = MST_ALLOCATE_NEW_PAYLOAD;
> > }
> >   
> > -   if (enable && drm_dp_add_payload_part2(mst_mgr, mst_state->base.state, 
> > payload)) {
> > +   if (enable && drm_dp_add_payload_part2(mst_mgr, mst_state->base.state, 
> > payload) == 0) {
> > amdgpu_dm_set_mst_status(&aconnector->mst_status,
> > set_flag, false);
> > } else {
> 
> Hi Lyude,
> 
> Maybe I'm missing something, but I can't find the 
> drm_dp_add_payload_part2() function on amd-staging-drm-next. Which repo 
> are you using?

If it's not on amd-staging-drm-next then it likely hasn't gotten backported to
amd's branch yet and is in drm-misc-next

> 
> Thanks
> Siqueira
> 

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat

Re: [PATCH v2 5/7] drm/msm/dsi: Account for DSC's bits_per_pixel having 4 fractional bits

2022-10-05 Thread Dmitry Baryshkov

On Wed, 5 Oct 2022 at 21:17, Marijn Suijten
 wrote:
>
> drm_dsc_config's bits_per_pixel field holds a fractional value with 4
> bits, which all panel drivers should adhere to for
> drm_dsc_pps_payload_pack() to generate a valid payload.  All code in the
> DSI driver here seems to assume that this field doesn't contain any
> fractional bits, hence resulting in the wrong values being computed.
> Since none of the calculations leave any room for fractional bits or
> seem to indicate any possible area of support, disallow such values
> altogether.
>
> Fixes: b9080324d6ca ("drm/msm/dsi: add support for dsc data")
> Signed-off-by: Marijn Suijten 
> ---
>  drivers/gpu/drm/msm/dsi/dsi_host.c | 25 +
>  1 file changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index f42794cdd4c1..4717d49d76be 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -33,7 +33,7 @@
>
>  #define DSI_RESET_TOGGLE_DELAY_MS 20
>
> -static int dsi_populate_dsc_params(struct drm_dsc_config *dsc);
> +static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct 
> drm_dsc_config *dsc);
>
>  static int dsi_get_version(const void __iomem *base, u32 *major, u32 *minor)
>  {
> @@ -908,6 +908,7 @@ static void dsi_timing_setup(struct msm_dsi_host 
> *msm_host, bool is_bonded_dsi)
> u32 va_end = va_start + mode->vdisplay;
> u32 hdisplay = mode->hdisplay;
> u32 wc;
> +   int ret;
>
> DBG("");
>
> @@ -943,7 +944,9 @@ static void dsi_timing_setup(struct msm_dsi_host 
> *msm_host, bool is_bonded_dsi)
> /* we do the calculations for dsc parameters here so that
>  * panel can use these parameters
>  */
> -   dsi_populate_dsc_params(dsc);
> +   ret = dsi_populate_dsc_params(msm_host, dsc);
> +   if (ret)
> +   return;
>
> /* Divide the display by 3 but keep back/font porch and
>  * pulse width same
> @@ -1769,7 +1772,7 @@ static char bpg_offset[DSC_NUM_BUF_RANGES] = {
> 2, 0, 0, -2, -4, -6, -8, -8, -8, -10, -10, -12, -12, -12, -12
>  };
>
> -static int dsi_populate_dsc_params(struct drm_dsc_config *dsc)
> +static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct 
> drm_dsc_config *dsc)
>  {
> int mux_words_size;
> int groups_per_line, groups_total;
> @@ -1780,6 +1783,12 @@ static int dsi_populate_dsc_params(struct 
> drm_dsc_config *dsc)
> int data;
> int final_value, final_scale;
> int i;
> +   u16 bpp = dsc->bits_per_pixel >> 4;
> +
> +   if (dsc->bits_per_pixel & 0xf) {
> +   DRM_DEV_ERROR(&msm_host->pdev->dev, "DSI does not support 
> fractional bits_per_pixel\n");
> +   return -EINVAL;
> +   }
>
> dsc->rc_model_size = 8192;
> dsc->first_line_bpg_offset = 12;
> @@ -1801,7 +1810,7 @@ static int dsi_populate_dsc_params(struct 
> drm_dsc_config *dsc)
> }
>
> dsc->initial_offset = 6144; /* Not bpp 12 */
> -   if (dsc->bits_per_pixel != 8)
> +   if (bpp != 8)
> dsc->initial_offset = 2048; /* bpp = 12 */
>
> mux_words_size = 48;/* bpc == 8/10 */
> @@ -1824,14 +1833,14 @@ static int dsi_populate_dsc_params(struct 
> drm_dsc_config *dsc)
>  * params are calculated
>  */
> groups_per_line = DIV_ROUND_UP(dsc->slice_width, 3);
> -   dsc->slice_chunk_size = DIV_ROUND_UP(dsc->slice_width * 
> dsc->bits_per_pixel, 8);
> +   dsc->slice_chunk_size = DIV_ROUND_UP(dsc->slice_width * bpp, 8);

I'd still prefer if we can get closer to drm_dsc_compute_rc_parameters().
The mentioned function has the following code:

vdsc_cfg->slice_chunk_size = DIV_ROUND_UP(vdsc_cfg->slice_width *

vdsc_cfg->bits_per_pixel,
  (8 * 16));

In fact, could you please take a look if we can switch to using this
function and drop our code?

>
> /* rbs-min */
> min_rate_buffer_size =  dsc->rc_model_size - dsc->initial_offset +
> -   dsc->initial_xmit_delay * dsc->bits_per_pixel 
> +
> +   dsc->initial_xmit_delay * bpp +
> groups_per_line * dsc->first_line_bpg_offset;
>
> -   hrd_delay = DIV_ROUND_UP(min_rate_buffer_size, dsc->bits_per_pixel);
> +   hrd_delay = DIV_ROUND_UP(min_rate_buffer_size, bpp);
>
> dsc->initial_dec_delay = hrd_delay - dsc->initial_xmit_delay;
>
> @@ -1854,7 +1863,7 @@ static int dsi_populate_dsc_params(struct 
> drm_dsc_config *dsc)
> data = 2048 * (dsc->rc_model_size - dsc->initial_offset + 
> num_extra_mux_bits);
> dsc->slice_bpg_offset = DIV_ROUND_UP(data, groups_total);
>
> -   data = dsc->initial_xmit_delay * dsc->bits_per_pixel;
> +   dat

Re: [Intel-gfx] [PATCH v4 3/4] drm/i915: Make the heartbeat play nice with long pre-emption timeouts

2022-10-05 Thread John Harrison


On 10/3/2022 05:00, Tvrtko Ursulin wrote:

On 03/10/2022 08:53, Tvrtko Ursulin wrote:

On 30/09/2022 18:44, John Harrison wrote:

On 9/30/2022 02:22, Tvrtko Ursulin wrote:

On 29/09/2022 17:21, John Harrison wrote:

On 9/29/2022 00:42, Tvrtko Ursulin wrote:

On 29/09/2022 03:18, john.c.harri...@intel.com wrote:

From: John Harrison 

Compute workloads are inherently not pre-emptible for long 
periods on

current hardware. As a workaround for this, the pre-emption timeout
for compute capable engines was disabled. This is undesirable 
with GuC
submission as it prevents per engine reset of hung contexts. 
Hence the

next patch will re-enable the timeout but bumped up by an order of
magnitude.

However, the heartbeat might not respect that. Depending upon 
current

activity, a pre-emption to the heartbeat pulse might not even be
attempted until the last heartbeat period. Which means that only 
one

period is granted for the pre-emption to occur. With the aforesaid
bump, the pre-emption timeout could be significantly larger than 
this

heartbeat period.

So adjust the heartbeat code to take the pre-emption timeout into
account. When it reaches the final (high priority) period, it now
ensures the delay before hitting reset is bigger than the 
pre-emption

timeout.

v2: Fix for selftests which adjust the heartbeat period manually.

Signed-off-by: John Harrison 
---
  .../gpu/drm/i915/gt/intel_engine_heartbeat.c  | 19 
+++

  1 file changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c 
b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c

index a3698f611f457..823a790a0e2ae 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
@@ -22,9 +22,28 @@
    static bool next_heartbeat(struct intel_engine_cs *engine)
  {
+    struct i915_request *rq;
  long delay;
    delay = READ_ONCE(engine->props.heartbeat_interval_ms);
+
+    rq = engine->heartbeat.systole;
+
+    if (rq && rq->sched.attr.priority >= I915_PRIORITY_BARRIER &&
+    delay == engine->defaults.heartbeat_interval_ms) {


Maybe I forgot but what is the reason for the check against the 
default heartbeat interval?
That's the 'v2: fix for selftests that manually adjust the 
heartbeat'. If something (or someone) has explicitly set an 
override of the heartbeat then it has to be assumed that they know 
what they are doing, and if things don't work any more that's 
their problem. But if we don't respect their override then they 
won't get the timings they expect and the selftest will fail.


Isn't this a bit too strict for the non-selftest case? If the new 
concept is extending the last pulse to guarantee preemption, then I 
think we could allow tweaking of the heartbeat period. Like what if 
user wants 1s, or 10s instead of 2.5s - why would that need to 
break the improvement from this patch?

Then the user is back to where they were before this patch.



In what ways selftests fail? Are they trying to guess time to reset 
based on the hearbeat period set? If so perhaps add a helper to 
query it based on the last pulse extension.


I don't recall. It was six months ago when I was actually working on 
this. And right now I do not have the time to go back and re-run all 
the testing and re-write a bunch of self tests with whole new 
helpers and algorithms and whatever else might be necessary to 
polish this to perfection. And in the meantime, all the existing 
issues are still present - there is no range checking on any of this 
stuff, it is very possible for a driver with default settings to 
break a legal workload because the heartbeat and pre-emption are 
fighting with each other, we don't even have per engine resets 
enabled, etc.


Maybe it could be even better with a follow up patch. Feel free to 
do that. But as it stands, this patch set significantly improves the 
situation without making anything worse.


As we seem to be in agreement that the check against default 
heartbeat is a hack with only purpose to work around assumptions made 
by selftests, then please file a Jira about removing it (this hack). 
Then work can be assigned to someone to clean it up. With that done I 
would agree the series is indeed an improvement and it would have my 
ack.

VLK-39595



One more thing - put a comment in the code along the lines of 
"FIXME/HACK: Work around selftests assumptions by only extending the 
last heartbeat if the period is at default value". The the Jira can 
associate to that comment.


Until that is resolve it may also be worth emitting a drm_notice if 
heartbeat is changed via sysfs? Informing users the things will not 
work as expected if they fiddle with it. Whether as a blanket warning 
or checking first the 3-4x heartbeat vs preempt timeout value. That 
message should then go away once the follow up work to fix the 
selftests is done. See what the other reviewers will think.


What should the drm_notice say? How can you describ

Re: [PATCH 4/5] drm/msm/dpu1: Account for DSC's bits_per_pixel having 4 fractional bits

On 2022-10-05 07:19:11, Abhinav Kumar wrote:
> > [..]
> > 
> > Or are you suggesting to "redo" the DSC integration work based on a
> > (much) newer display techpack (SDE driver)?
> 
> There is no need to redo the DSC integration now.
> 
> The code I am referring to is here :
> 
> https://git.codelinaro.org/clo/la/platform/vendor/opensource/display-drivers/-/blob/DISPLAY.LA.2.0.r1-08000-WAIPIO.0/msm/sde_dsc_helper.c#L240
> 
> So with respect to the redundant math in patches 1/3/4/5 of this series, 
> I dont see all the redundant math anymore in this calculation.
> 
> This is what i meant by my comment.

It all seems to have had a nice clean-up.  What I meant is that it might
have been more efficient to copy-paste the cleaned-up, improved
downstream implementation instead of individually trying to find and
address all issues; either by running into these bugs on upstream (the
way this patch series came to be), or by comparing the new/improved
downstream with upstream.

> When DSC changes were pushed, they were indeed validated on sdm845 
> devices by Vinod so there was a certain level of confidence on those 
> changes.

Some branches seemed to have a display driver without the DCS PPS
command, or with the command commented out (relying on the panel being
configured for DSC by the bootloader).  The "4 fractional bits" issue
might have gone unnoticed since the panel driver was writing, and both
the DSI and DPU1 drivers were reading this field without those
fractional bits.

It's only a small bug (but with disastrous results on panel drivers with
proper DCS PPS command), the rest is cruft that was copied from
downstream but not filtered out in development nor review.

> At this point, we should just consider these as bug-fixes for upstream 
> and keep going. A full redo is not required.

Ack, at least that doesn't make this series/work obsolete :)

> At some point in the next couple of months, we plan to add DSC 1.2 
> support to MSM.

That's appreciated as all devices I have here (on newer SoCs with DSC
1.2) also have high-resolution, high-fps panels that need DSC to
function correctly.
We'll see who gets to it first though :)

> We will check for any missing changes (if any after this series of 
> yours) and push those as part of that.

There are a few, but it's hard to say until the panel is fully working.
Current focus is on sm8250.
We can discuss this at a more informal pace in #linux-msm if you're
interested.

- Marijn

Re: [git pull] drm for 6.1-rc1

2022-10-05 Thread pr-tracker-bot

The pull request you sent on Wed, 5 Oct 2022 13:41:47 +1000:

> git://anongit.freedesktop.org/drm/drm tags/drm-next-2022-10-05

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/7e6739b9336e61fe23ca4e2c8d1fda8f19f979bf

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html

Re: [git pull] drm for 6.1-rc1

2022-10-05 Thread Linus Torvalds

On Tue, Oct 4, 2022 at 8:42 PM Dave Airlie  wrote:
>
> This is very conflict heavy, mostly the correct answer is picking
> the version from drm-next.

Ugh, yes, that was a bit annoying.

I get the same end result as you did, but I do wonder if the drm
people should try to keep some kind of separate "fixes" branches for
things that go both into the development tree and then get sent to me
for fixes pulls?

Hopefully this "lots of pointless noise" was a one-off, but it might
be due to how you guys work..

  Linus

Re: [PATCH 5/5] drm/dsc: Prevent negative BPG offsets from shadowing adjacent bitfields

On 2022-10-05 08:33:23, Abhinav Kumar wrote:
[..]
> hmm  downstream seems to have the & just before the reg write
> 
> https://git.codelinaro.org/clo/la/platform/vendor/opensource/display-drivers/-/blob/DISPLAY.LA.2.0.r1-08000-WAIPIO.0/msm/sde/sde_hw_dsc_1_2.c#L310

https://git.codelinaro.org/clo/la/platform/vendor/opensource/display-drivers/-/blob/DISPLAY.LA.2.0.r1-08000-WAIPIO.0/msm/sde/sde_hw_dsc.c#L156

The reference code for NON-v1.2 doesn't do this here, but you already
confirmed by the docs - and I confirmed by testing a set of size #1 -
that the register is fine with having only 6 bits.

> But yes, we can move this to the dsi calculation to match what others 
> are doing. I am fine with that.

Thanks, done that in v2.

> > [..]
> 
> Even I wasnt. When I was reviewing this series, it seemed like a valid 
> change so I checked the latest downstream code like i always do to see 
> whether and how this is handled and found that these issues were 
> addressed there so thought i would update that here.

Thanks for confirming that it was done in the correct/same way :)

> > [..]
> Its not really parallel development OR competing drivers.
> The design of these features downstream (not just DSC but many others) 
> is quite different because some of the downstream designs wont get 
> accepted upstream as its tightly coupled with our 
> compositor/device-tree. Thats where we are slowly trying to implement 
> these in an upstream compatible way.

But this is what it feels like.

To me this sounds like downstream is more of a staging / prototyping
area that is actively used in production, while the driver
implementation is fleshed out and slowly brought to mainline in a
careful and controlled manner.

That's not a bad thing, but it does mean that mainline always lags
behind in terms of features and hardware support.  At least I'm glad
to hear that downstream is slowly using more DRM primitives, and the
driver is becoming more digestible as well.

> BTW, that being said. Its not always the case that every issue seen 
> upstream has already been fixed downstream. In fact, on some occasions 
> we found something fixed in upstream in a better way and ported them 
> downstream too.

I wasn't expecting anything else, as different drivers have inevitably
different details and different bugs.  The issues this series fixes
weren't applicable to the downstream kernel because it (at the time)
wasn't even using this drm_dsc_config struct with different semantics.
Regardless, it's good to hear that fixes are transplanted in both ways
even if it does mean extra overhead maintaining and keeping tabs on two
drivers at once.

- Marijn

[PATCH v2 6/7] drm/msm/dpu1: Account for DSC's bits_per_pixel having 4 fractional bits

According to the comment this DPU register contains the bits per pixel
as a 6.4 fractional value, conveniently matching the contents of
bits_per_pixel in struct drm_dsc_config which also uses 4 fractional
bits.  However, the downstream source this implementation was
copy-pasted from has its bpp field stored _without_ fractional part.

This makes the entire convoluted math obsolete as it is impossible to
pull those 4 fractional bits out of thin air, by somehow trying to reuse
the lowest 2 bits of a non-fractional bpp (lsb = bpp % 4??).

The rest of the code merely attempts to keep the integer part a multiple
of 4, which is rendered useless thanks to data |= dsc->bits_per_pixel <<
12; already filling up those bits anyway (but not on downstream).

Fixes: c110cfd1753e ("drm/msm/disp/dpu1: Add support for DSC")
Reviewed-by: Abhinav Kumar 
Reviewed-by: Dmitry Baryshkov 
Reviewed-by: Vinod Koul 
Signed-off-by: Marijn Suijten 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 11 ++-
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
index 46cc2afd2bb9..c63e6eef1ba6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
@@ -47,7 +47,7 @@ static void dpu_hw_dsc_config(struct dpu_hw_dsc *hw_dsc,
  u32 initial_lines)
 {
struct dpu_hw_blk_reg_map *c = &hw_dsc->hw;
-   u32 data, lsb, bpp;
+   u32 data;
u32 slice_last_group_size;
u32 det_thresh_flatness;
bool is_cmd_mode = !(mode & DSC_MODE_VIDEO);
@@ -61,14 +61,7 @@ static void dpu_hw_dsc_config(struct dpu_hw_dsc *hw_dsc,
data = (initial_lines << 20);
data |= ((slice_last_group_size - 1) << 18);
/* bpp is 6.4 format, 4 LSBs bits are for fractional part */
-   data |= dsc->bits_per_pixel << 12;
-   lsb = dsc->bits_per_pixel % 4;
-   bpp = dsc->bits_per_pixel / 4;
-   bpp *= 4;
-   bpp <<= 4;
-   bpp |= lsb;
-
-   data |= bpp << 8;
+   data |= (dsc->bits_per_pixel << 8);
data |= (dsc->block_pred_enable << 7);
data |= (dsc->line_buf_depth << 3);
data |= (dsc->simple_422 << 2);
-- 
2.38.0

[PATCH v2 7/7] drm/msm/dsi: Prevent signed BPG offsets from bleeding into adjacent bits

The bpg_offset array contains negative BPG offsets which fill the full 8
bits of a char thanks to two's complement: this however results in those
bits bleeding into the next field when the value is packed into DSC PPS
by the drm_dsc_helper function, which only expects range_bpg_offset to
contain 6-bit wide values.  As a consequence random slices appear
corrupted on-screen (tested on a Sony Tama Akatsuki device with sdm845).

Use AND operators to limit these two's complement values to 6 bits,
similar to the AMD and i915 drivers.

Fixes: b9080324d6ca ("drm/msm/dsi: add support for dsc data")
Signed-off-by: Marijn Suijten 
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 4717d49d76be..b3cff3d3aa85 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -1806,7 +1806,11 @@ static int dsi_populate_dsc_params(struct msm_dsi_host 
*msm_host, struct drm_dsc
for (i = 0; i < DSC_NUM_BUF_RANGES; i++) {
dsc->rc_range_params[i].range_min_qp = min_qp[i];
dsc->rc_range_params[i].range_max_qp = max_qp[i];
-   dsc->rc_range_params[i].range_bpg_offset = bpg_offset[i];
+   /*
+* Range BPG Offset contains two's-complement signed values 
that fill
+* 8 bits, yet the registers and DCS PPS field are only 6 bits 
wide.
+*/
+   dsc->rc_range_params[i].range_bpg_offset = bpg_offset[i] & 
DSC_RANGE_BPG_OFFSET_MASK;
}
 
dsc->initial_offset = 6144; /* Not bpp 12 */
-- 
2.38.0

[PATCH v2 4/7] drm/msm/dsi: Reuse earlier computed dsc->slice_chunk_size

dsi_populate_dsc_params() is called prior to dsi_update_dsc_timing() and
already computes a value for slice_chunk_size, whose value doesn't need
to be recomputed and re-set here.

Fixes: 08802f515c3c ("drm/msm/dsi: Add support for DSC configuration")
Signed-off-by: Marijn Suijten 
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 48c966375ffa..f42794cdd4c1 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -845,7 +845,6 @@ static void dsi_update_dsc_timing(struct msm_dsi_host 
*msm_host, bool is_cmd_mod
u32 reg, reg_ctrl, reg_ctrl2;
u32 slice_per_intf, total_bytes_per_intf;
u32 pkt_per_line;
-   u32 bytes_in_slice;
u32 eol_byte_num;
 
/* first calculate dsc parameters and then program
@@ -860,11 +859,7 @@ static void dsi_update_dsc_timing(struct msm_dsi_host 
*msm_host, bool is_cmd_mod
if (slice_per_intf > dsc->slice_count)
dsc->slice_count = 1;
 
-   bytes_in_slice = DIV_ROUND_UP(dsc->slice_width * dsc->bits_per_pixel, 
8);
-
-   dsc->slice_chunk_size = bytes_in_slice;
-
-   total_bytes_per_intf = bytes_in_slice * slice_per_intf;
+   total_bytes_per_intf = dsc->slice_chunk_size * slice_per_intf;
 
eol_byte_num = total_bytes_per_intf % 3;
pkt_per_line = slice_per_intf / dsc->slice_count;
@@ -890,7 +885,7 @@ static void dsi_update_dsc_timing(struct msm_dsi_host 
*msm_host, bool is_cmd_mod
reg_ctrl |= reg;
 
reg_ctrl2 &= 
~DSI_COMMAND_COMPRESSION_MODE_CTRL2_STREAM0_SLICE_WIDTH__MASK;
-   reg_ctrl2 |= 
DSI_COMMAND_COMPRESSION_MODE_CTRL2_STREAM0_SLICE_WIDTH(bytes_in_slice);
+   reg_ctrl2 |= 
DSI_COMMAND_COMPRESSION_MODE_CTRL2_STREAM0_SLICE_WIDTH(dsc->slice_chunk_size);
 
dsi_write(msm_host, REG_DSI_COMMAND_COMPRESSION_MODE_CTRL, 
reg_ctrl);
dsi_write(msm_host, REG_DSI_COMMAND_COMPRESSION_MODE_CTRL2, 
reg_ctrl2);
-- 
2.38.0

[PATCH v2 5/7] drm/msm/dsi: Account for DSC's bits_per_pixel having 4 fractional bits

drm_dsc_config's bits_per_pixel field holds a fractional value with 4
bits, which all panel drivers should adhere to for
drm_dsc_pps_payload_pack() to generate a valid payload.  All code in the
DSI driver here seems to assume that this field doesn't contain any
fractional bits, hence resulting in the wrong values being computed.
Since none of the calculations leave any room for fractional bits or
seem to indicate any possible area of support, disallow such values
altogether.

Fixes: b9080324d6ca ("drm/msm/dsi: add support for dsc data")
Signed-off-by: Marijn Suijten 
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 25 +
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index f42794cdd4c1..4717d49d76be 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -33,7 +33,7 @@
 
 #define DSI_RESET_TOGGLE_DELAY_MS 20
 
-static int dsi_populate_dsc_params(struct drm_dsc_config *dsc);
+static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct 
drm_dsc_config *dsc);
 
 static int dsi_get_version(const void __iomem *base, u32 *major, u32 *minor)
 {
@@ -908,6 +908,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, 
bool is_bonded_dsi)
u32 va_end = va_start + mode->vdisplay;
u32 hdisplay = mode->hdisplay;
u32 wc;
+   int ret;
 
DBG("");
 
@@ -943,7 +944,9 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, 
bool is_bonded_dsi)
/* we do the calculations for dsc parameters here so that
 * panel can use these parameters
 */
-   dsi_populate_dsc_params(dsc);
+   ret = dsi_populate_dsc_params(msm_host, dsc);
+   if (ret)
+   return;
 
/* Divide the display by 3 but keep back/font porch and
 * pulse width same
@@ -1769,7 +1772,7 @@ static char bpg_offset[DSC_NUM_BUF_RANGES] = {
2, 0, 0, -2, -4, -6, -8, -8, -8, -10, -10, -12, -12, -12, -12
 };
 
-static int dsi_populate_dsc_params(struct drm_dsc_config *dsc)
+static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct 
drm_dsc_config *dsc)
 {
int mux_words_size;
int groups_per_line, groups_total;
@@ -1780,6 +1783,12 @@ static int dsi_populate_dsc_params(struct drm_dsc_config 
*dsc)
int data;
int final_value, final_scale;
int i;
+   u16 bpp = dsc->bits_per_pixel >> 4;
+
+   if (dsc->bits_per_pixel & 0xf) {
+   DRM_DEV_ERROR(&msm_host->pdev->dev, "DSI does not support 
fractional bits_per_pixel\n");
+   return -EINVAL;
+   }
 
dsc->rc_model_size = 8192;
dsc->first_line_bpg_offset = 12;
@@ -1801,7 +1810,7 @@ static int dsi_populate_dsc_params(struct drm_dsc_config 
*dsc)
}
 
dsc->initial_offset = 6144; /* Not bpp 12 */
-   if (dsc->bits_per_pixel != 8)
+   if (bpp != 8)
dsc->initial_offset = 2048; /* bpp = 12 */
 
mux_words_size = 48;/* bpc == 8/10 */
@@ -1824,14 +1833,14 @@ static int dsi_populate_dsc_params(struct 
drm_dsc_config *dsc)
 * params are calculated
 */
groups_per_line = DIV_ROUND_UP(dsc->slice_width, 3);
-   dsc->slice_chunk_size = DIV_ROUND_UP(dsc->slice_width * 
dsc->bits_per_pixel, 8);
+   dsc->slice_chunk_size = DIV_ROUND_UP(dsc->slice_width * bpp, 8);
 
/* rbs-min */
min_rate_buffer_size =  dsc->rc_model_size - dsc->initial_offset +
-   dsc->initial_xmit_delay * dsc->bits_per_pixel +
+   dsc->initial_xmit_delay * bpp +
groups_per_line * dsc->first_line_bpg_offset;
 
-   hrd_delay = DIV_ROUND_UP(min_rate_buffer_size, dsc->bits_per_pixel);
+   hrd_delay = DIV_ROUND_UP(min_rate_buffer_size, bpp);
 
dsc->initial_dec_delay = hrd_delay - dsc->initial_xmit_delay;
 
@@ -1854,7 +1863,7 @@ static int dsi_populate_dsc_params(struct drm_dsc_config 
*dsc)
data = 2048 * (dsc->rc_model_size - dsc->initial_offset + 
num_extra_mux_bits);
dsc->slice_bpg_offset = DIV_ROUND_UP(data, groups_total);
 
-   data = dsc->initial_xmit_delay * dsc->bits_per_pixel;
+   data = dsc->initial_xmit_delay * bpp;
final_value =  dsc->rc_model_size - data + num_extra_mux_bits;
dsc->final_offset = final_value;
 
-- 
2.38.0

[PATCH v2 3/7] drm/msm/dsi: Use DIV_ROUND_UP instead of conditional increment on modulo

This exact same math is used to compute bytes_in_slice above in
dsi_update_dsc_timing(), also used to fill slice_chunk_size.

Fixes: b9080324d6ca ("drm/msm/dsi: add support for dsc data")
Signed-off-by: Marijn Suijten 
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index c746ed5d61f9..48c966375ffa 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -1829,9 +1829,7 @@ static int dsi_populate_dsc_params(struct drm_dsc_config 
*dsc)
 * params are calculated
 */
groups_per_line = DIV_ROUND_UP(dsc->slice_width, 3);
-   dsc->slice_chunk_size = dsc->slice_width * dsc->bits_per_pixel / 8;
-   if ((dsc->slice_width * dsc->bits_per_pixel) % 8)
-   dsc->slice_chunk_size++;
+   dsc->slice_chunk_size = DIV_ROUND_UP(dsc->slice_width * 
dsc->bits_per_pixel, 8);
 
/* rbs-min */
min_rate_buffer_size =  dsc->rc_model_size - dsc->initial_offset +
-- 
2.38.0

[PATCH v2 2/7] drm/msm/dsi: Remove repeated calculation of slice_per_intf

slice_per_intf is already computed for intf_width, which holds the same
value as hdisplay.

Fixes: 08802f515c3c ("drm/msm/dsi: Add support for DSC configuration")
Reviewed-by: Bjorn Andersson 
Reviewed-by: Konrad Dybcio 
Reviewed-by: Abhinav Kumar 
Reviewed-by: Vinod Koul 
Signed-off-by: Marijn Suijten 
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 70077d1f0f21..c746ed5d61f9 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -842,7 +842,7 @@ static void dsi_ctrl_config(struct msm_dsi_host *msm_host, 
bool enable,
 static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool 
is_cmd_mode, u32 hdisplay)
 {
struct drm_dsc_config *dsc = msm_host->dsc;
-   u32 reg, intf_width, reg_ctrl, reg_ctrl2;
+   u32 reg, reg_ctrl, reg_ctrl2;
u32 slice_per_intf, total_bytes_per_intf;
u32 pkt_per_line;
u32 bytes_in_slice;
@@ -851,8 +851,7 @@ static void dsi_update_dsc_timing(struct msm_dsi_host 
*msm_host, bool is_cmd_mod
/* first calculate dsc parameters and then program
 * compress mode registers
 */
-   intf_width = hdisplay;
-   slice_per_intf = DIV_ROUND_UP(intf_width, dsc->slice_width);
+   slice_per_intf = DIV_ROUND_UP(hdisplay, dsc->slice_width);
 
/* If slice_per_pkt is greater than slice_per_intf
 * then default to 1. This can happen during partial
@@ -861,7 +860,6 @@ static void dsi_update_dsc_timing(struct msm_dsi_host 
*msm_host, bool is_cmd_mod
if (slice_per_intf > dsc->slice_count)
dsc->slice_count = 1;
 
-   slice_per_intf = DIV_ROUND_UP(hdisplay, dsc->slice_width);
bytes_in_slice = DIV_ROUND_UP(dsc->slice_width * dsc->bits_per_pixel, 
8);
 
dsc->slice_chunk_size = bytes_in_slice;
-- 
2.38.0

[PATCH v2 0/7] drm/msm: Fix math issues in MSM DSC implementation

Various removals of complex yet unnecessary math, fixing all uses of
drm_dsc_config::bits_per_pixel to deal with the fact that this field
includes four fractional bits, and finally making sure that
range_bpg_offset contains values 6-bits wide to prevent overflows in
drm_dsc_pps_payload_pack().

Altogether this series is responsible for solving _all_ Display Stream
Compression issues and artifacts on the Sony Tama (sdm845) Akatsuki
smartphone (2880x1440p).

Changes since v1:

- Propagate r-b's, except (obviously) in patches that were (heavily)
  modified;
- Remove accidental debug code in dsi_cmd_dma_add;
- Move Range BPG Offset masking out of DCS PPS packing, back into the
  DSI driver when it is assigned to drm_dsc_config (this series is now
  strictly focusing on drm/msm again);
- Replace modulo-check resulting in conditional increment with
  DIV_ROUND_UP;
- Remove repeated calculation of slice_chunk_size;
- Use u16 instead of int when handling bits_per_pixel;
- Use DRM_DEV_ERROR instead of pr_err in DSI code;
- Also remove redundant target_bpp_x16 variable.

v1: 
https://lore.kernel.org/linux-arm-msm/20221001190807.358691-1-marijn.suij...@somainline.org/T/#u

Marijn Suijten (7):
  drm/msm/dsi: Remove useless math in DSC calculations
  drm/msm/dsi: Remove repeated calculation of slice_per_intf
  drm/msm/dsi: Use DIV_ROUND_UP instead of conditional increment on
modulo
  drm/msm/dsi: Reuse earlier computed dsc->slice_chunk_size
  drm/msm/dsi: Account for DSC's bits_per_pixel having 4 fractional bits
  drm/msm/dpu1: Account for DSC's bits_per_pixel having 4 fractional
bits
  drm/msm/dsi: Prevent signed BPG offsets from bleeding into adjacent
bits

 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 11 +
 drivers/gpu/drm/msm/dsi/dsi_host.c | 56 ++
 2 files changed, 28 insertions(+), 39 deletions(-)

--
2.38.0

[PATCH v2 1/7] drm/msm/dsi: Remove useless math in DSC calculations

Multiplying a value by 2 and adding 1 to it always results in a value
that is uneven, and that 1 gets truncated immediately when performing
integer division by 2 again.  There is no "rounding" possible here.

After that target_bpp_x16 is used to store a multiplication of
bits_per_pixel by 16 which is only ever read to immediately be divided
by 16 again, and is elided in much the same way.

Fixes: b9080324d6ca ("drm/msm/dsi: add support for dsc data")
Signed-off-by: Marijn Suijten 
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 8e4bc586c262..70077d1f0f21 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -1784,7 +1784,6 @@ static int dsi_populate_dsc_params(struct drm_dsc_config 
*dsc)
int hrd_delay;
int pre_num_extra_mux_bits, num_extra_mux_bits;
int slice_bits;
-   int target_bpp_x16;
int data;
int final_value, final_scale;
int i;
@@ -1864,14 +1863,7 @@ static int dsi_populate_dsc_params(struct drm_dsc_config 
*dsc)
data = 2048 * (dsc->rc_model_size - dsc->initial_offset + 
num_extra_mux_bits);
dsc->slice_bpg_offset = DIV_ROUND_UP(data, groups_total);
 
-   /* bpp * 16 + 0.5 */
-   data = dsc->bits_per_pixel * 16;
-   data *= 2;
-   data++;
-   data /= 2;
-   target_bpp_x16 = data;
-
-   data = (dsc->initial_xmit_delay * target_bpp_x16) / 16;
+   data = dsc->initial_xmit_delay * dsc->bits_per_pixel;
final_value =  dsc->rc_model_size - data + num_extra_mux_bits;
dsc->final_offset = final_value;
 
-- 
2.38.0

Re: [Intel-gfx] [PATCH v3 1/2] drm/i915: Add a wrapper for frequency debugfs

On Wed, 05 Oct 2022, Jani Nikula  wrote:
> On Wed, 05 Oct 2022, Vinay Belgaumkar  wrote:
>> Move it to the RPS source file.
>>
>> v2: Separate out code movement and functional changes (Jani)
>>
>> Signed-off-by: Vinay Belgaumkar 
>
> Reviewed-by: Jani Nikula 

PS. Sorry, I'll leave patch 2 for someone else to review. Thanks for
making the changes, patch 1 was trivial now. :)

>
>> ---
>>  drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 157 +
>>  drivers/gpu/drm/i915/gt/intel_rps.c   | 163 ++
>>  drivers/gpu/drm/i915/gt/intel_rps.h   |   3 +
>>  3 files changed, 167 insertions(+), 156 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c 
>> b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
>> index 10f680dbd7b6..40d0a3be42ac 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
>> @@ -344,162 +344,7 @@ void intel_gt_pm_frequency_dump(struct intel_gt *gt, 
>> struct drm_printer *p)
>>  drm_printf(p, "efficient (RPe) frequency: %d MHz\n",
>> intel_gpu_freq(rps, rps->efficient_freq));
>>  } else if (GRAPHICS_VER(i915) >= 6) {
>> -u32 rp_state_limits;
>> -u32 gt_perf_status;
>> -struct intel_rps_freq_caps caps;
>> -u32 rpmodectl, rpinclimit, rpdeclimit;
>> -u32 rpstat, cagf, reqf;
>> -u32 rpcurupei, rpcurup, rpprevup;
>> -u32 rpcurdownei, rpcurdown, rpprevdown;
>> -u32 rpupei, rpupt, rpdownei, rpdownt;
>> -u32 pm_ier, pm_imr, pm_isr, pm_iir, pm_mask;
>> -
>> -rp_state_limits = intel_uncore_read(uncore, 
>> GEN6_RP_STATE_LIMITS);
>> -gen6_rps_get_freq_caps(rps, &caps);
>> -if (IS_GEN9_LP(i915))
>> -gt_perf_status = intel_uncore_read(uncore, 
>> BXT_GT_PERF_STATUS);
>> -else
>> -gt_perf_status = intel_uncore_read(uncore, 
>> GEN6_GT_PERF_STATUS);
>> -
>> -/* RPSTAT1 is in the GT power well */
>> -intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL);
>> -
>> -reqf = intel_uncore_read(uncore, GEN6_RPNSWREQ);
>> -if (GRAPHICS_VER(i915) >= 9) {
>> -reqf >>= 23;
>> -} else {
>> -reqf &= ~GEN6_TURBO_DISABLE;
>> -if (IS_HASWELL(i915) || IS_BROADWELL(i915))
>> -reqf >>= 24;
>> -else
>> -reqf >>= 25;
>> -}
>> -reqf = intel_gpu_freq(rps, reqf);
>> -
>> -rpmodectl = intel_uncore_read(uncore, GEN6_RP_CONTROL);
>> -rpinclimit = intel_uncore_read(uncore, GEN6_RP_UP_THRESHOLD);
>> -rpdeclimit = intel_uncore_read(uncore, GEN6_RP_DOWN_THRESHOLD);
>> -
>> -rpstat = intel_uncore_read(uncore, GEN6_RPSTAT1);
>> -rpcurupei = intel_uncore_read(uncore, GEN6_RP_CUR_UP_EI) & 
>> GEN6_CURICONT_MASK;
>> -rpcurup = intel_uncore_read(uncore, GEN6_RP_CUR_UP) & 
>> GEN6_CURBSYTAVG_MASK;
>> -rpprevup = intel_uncore_read(uncore, GEN6_RP_PREV_UP) & 
>> GEN6_CURBSYTAVG_MASK;
>> -rpcurdownei = intel_uncore_read(uncore, GEN6_RP_CUR_DOWN_EI) & 
>> GEN6_CURIAVG_MASK;
>> -rpcurdown = intel_uncore_read(uncore, GEN6_RP_CUR_DOWN) & 
>> GEN6_CURBSYTAVG_MASK;
>> -rpprevdown = intel_uncore_read(uncore, GEN6_RP_PREV_DOWN) & 
>> GEN6_CURBSYTAVG_MASK;
>> -
>> -rpupei = intel_uncore_read(uncore, GEN6_RP_UP_EI);
>> -rpupt = intel_uncore_read(uncore, GEN6_RP_UP_THRESHOLD);
>> -
>> -rpdownei = intel_uncore_read(uncore, GEN6_RP_DOWN_EI);
>> -rpdownt = intel_uncore_read(uncore, GEN6_RP_DOWN_THRESHOLD);
>> -
>> -cagf = intel_rps_read_actual_frequency(rps);
>> -
>> -intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL);
>> -
>> -if (GRAPHICS_VER(i915) >= 11) {
>> -pm_ier = intel_uncore_read(uncore, 
>> GEN11_GPM_WGBOXPERF_INTR_ENABLE);
>> -pm_imr = intel_uncore_read(uncore, 
>> GEN11_GPM_WGBOXPERF_INTR_MASK);
>> -/*
>> - * The equivalent to the PM ISR & IIR cannot be read
>> - * without affecting the current state of the system
>> - */
>> -pm_isr = 0;
>> -pm_iir = 0;
>> -} else if (GRAPHICS_VER(i915) >= 8) {
>> -pm_ier = intel_uncore_read(uncore, GEN8_GT_IER(2));
>> -pm_imr = intel_uncore_read(uncore, GEN8_GT_IMR(2));
>> -pm_isr = intel_uncore_read(uncore, GEN8_GT_ISR(2));
>> -pm_iir = intel_uncore_read(uncore, GEN8_GT_IIR(2));
>> -} else {
>> -pm_ier = intel_uncore_read(uncore, GEN6_PMIER);
>> -pm_i

Re: [Intel-gfx] [PATCH v3 1/2] drm/i915: Add a wrapper for frequency debugfs

On Wed, 05 Oct 2022, Vinay Belgaumkar  wrote:
> Move it to the RPS source file.
>
> v2: Separate out code movement and functional changes (Jani)
>
> Signed-off-by: Vinay Belgaumkar 

Reviewed-by: Jani Nikula 

> ---
>  drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 157 +
>  drivers/gpu/drm/i915/gt/intel_rps.c   | 163 ++
>  drivers/gpu/drm/i915/gt/intel_rps.h   |   3 +
>  3 files changed, 167 insertions(+), 156 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c 
> b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> index 10f680dbd7b6..40d0a3be42ac 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
> @@ -344,162 +344,7 @@ void intel_gt_pm_frequency_dump(struct intel_gt *gt, 
> struct drm_printer *p)
>   drm_printf(p, "efficient (RPe) frequency: %d MHz\n",
>  intel_gpu_freq(rps, rps->efficient_freq));
>   } else if (GRAPHICS_VER(i915) >= 6) {
> - u32 rp_state_limits;
> - u32 gt_perf_status;
> - struct intel_rps_freq_caps caps;
> - u32 rpmodectl, rpinclimit, rpdeclimit;
> - u32 rpstat, cagf, reqf;
> - u32 rpcurupei, rpcurup, rpprevup;
> - u32 rpcurdownei, rpcurdown, rpprevdown;
> - u32 rpupei, rpupt, rpdownei, rpdownt;
> - u32 pm_ier, pm_imr, pm_isr, pm_iir, pm_mask;
> -
> - rp_state_limits = intel_uncore_read(uncore, 
> GEN6_RP_STATE_LIMITS);
> - gen6_rps_get_freq_caps(rps, &caps);
> - if (IS_GEN9_LP(i915))
> - gt_perf_status = intel_uncore_read(uncore, 
> BXT_GT_PERF_STATUS);
> - else
> - gt_perf_status = intel_uncore_read(uncore, 
> GEN6_GT_PERF_STATUS);
> -
> - /* RPSTAT1 is in the GT power well */
> - intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL);
> -
> - reqf = intel_uncore_read(uncore, GEN6_RPNSWREQ);
> - if (GRAPHICS_VER(i915) >= 9) {
> - reqf >>= 23;
> - } else {
> - reqf &= ~GEN6_TURBO_DISABLE;
> - if (IS_HASWELL(i915) || IS_BROADWELL(i915))
> - reqf >>= 24;
> - else
> - reqf >>= 25;
> - }
> - reqf = intel_gpu_freq(rps, reqf);
> -
> - rpmodectl = intel_uncore_read(uncore, GEN6_RP_CONTROL);
> - rpinclimit = intel_uncore_read(uncore, GEN6_RP_UP_THRESHOLD);
> - rpdeclimit = intel_uncore_read(uncore, GEN6_RP_DOWN_THRESHOLD);
> -
> - rpstat = intel_uncore_read(uncore, GEN6_RPSTAT1);
> - rpcurupei = intel_uncore_read(uncore, GEN6_RP_CUR_UP_EI) & 
> GEN6_CURICONT_MASK;
> - rpcurup = intel_uncore_read(uncore, GEN6_RP_CUR_UP) & 
> GEN6_CURBSYTAVG_MASK;
> - rpprevup = intel_uncore_read(uncore, GEN6_RP_PREV_UP) & 
> GEN6_CURBSYTAVG_MASK;
> - rpcurdownei = intel_uncore_read(uncore, GEN6_RP_CUR_DOWN_EI) & 
> GEN6_CURIAVG_MASK;
> - rpcurdown = intel_uncore_read(uncore, GEN6_RP_CUR_DOWN) & 
> GEN6_CURBSYTAVG_MASK;
> - rpprevdown = intel_uncore_read(uncore, GEN6_RP_PREV_DOWN) & 
> GEN6_CURBSYTAVG_MASK;
> -
> - rpupei = intel_uncore_read(uncore, GEN6_RP_UP_EI);
> - rpupt = intel_uncore_read(uncore, GEN6_RP_UP_THRESHOLD);
> -
> - rpdownei = intel_uncore_read(uncore, GEN6_RP_DOWN_EI);
> - rpdownt = intel_uncore_read(uncore, GEN6_RP_DOWN_THRESHOLD);
> -
> - cagf = intel_rps_read_actual_frequency(rps);
> -
> - intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL);
> -
> - if (GRAPHICS_VER(i915) >= 11) {
> - pm_ier = intel_uncore_read(uncore, 
> GEN11_GPM_WGBOXPERF_INTR_ENABLE);
> - pm_imr = intel_uncore_read(uncore, 
> GEN11_GPM_WGBOXPERF_INTR_MASK);
> - /*
> -  * The equivalent to the PM ISR & IIR cannot be read
> -  * without affecting the current state of the system
> -  */
> - pm_isr = 0;
> - pm_iir = 0;
> - } else if (GRAPHICS_VER(i915) >= 8) {
> - pm_ier = intel_uncore_read(uncore, GEN8_GT_IER(2));
> - pm_imr = intel_uncore_read(uncore, GEN8_GT_IMR(2));
> - pm_isr = intel_uncore_read(uncore, GEN8_GT_ISR(2));
> - pm_iir = intel_uncore_read(uncore, GEN8_GT_IIR(2));
> - } else {
> - pm_ier = intel_uncore_read(uncore, GEN6_PMIER);
> - pm_imr = intel_uncore_read(uncore, GEN6_PMIMR);
> - pm_isr = intel_uncore_read(uncore, GEN6_PMISR);
> - pm_iir = intel_uncore_read(uncore, GEN6_PMIIR);
> - }

Re: [PATCH v2] drm/ssd130x: Iterate over damage clips instead of using a merged rect




Am 30.09.22 um 17:29 schrieb Javier Martinez Canillas:

The drm_atomic_helper_damage_merged() helper merges all the damage clips
into one rectangle. If there are multiple damage clips that aren't close
to each other, the resulting rectangle could be quite big.

Instead of using that function helper, iterate over all the damage clips
and update them one by one.

Suggested-by: Jocelyn Falempe 
Signed-off-by: Javier Martinez Canillas 


Acked-by: Thomas Zimmermann 


---

Changes in v2:
- Move the dst_clip assignment inside the drm_atomic_for_each_plane_damage()
   loop (Thomas Zimmermann).
- Pass dst_clip instead of damage area as argument to ssd130x_fb_blit_rect()
   function (Thomas Zimmermann)

  drivers/gpu/drm/solomon/ssd130x.c | 19 +++
  1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/solomon/ssd130x.c 
b/drivers/gpu/drm/solomon/ssd130x.c
index bc41a5ae810a..f456b233d2e7 100644
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd130x.c
@@ -578,21 +578,24 @@ static void 
ssd130x_primary_plane_helper_atomic_update(struct drm_plane *plane,
struct drm_plane_state *plane_state = 
drm_atomic_get_new_plane_state(state, plane);
struct drm_plane_state *old_plane_state = 
drm_atomic_get_old_plane_state(state, plane);
struct drm_shadow_plane_state *shadow_plane_state = 
to_drm_shadow_plane_state(plane_state);
+   struct drm_atomic_helper_damage_iter iter;
struct drm_device *drm = plane->dev;
-   struct drm_rect src_clip, dst_clip;
+   struct drm_rect dst_clip;
+   struct drm_rect damage;
int idx;
  
-	if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &src_clip))

+   if (!drm_dev_enter(drm, &idx))
return;
  
-	dst_clip = plane_state->dst;

-   if (!drm_rect_intersect(&dst_clip, &src_clip))
-   return;
+   drm_atomic_helper_damage_iter_init(&iter, old_plane_state, plane_state);
+   drm_atomic_for_each_plane_damage(&iter, &damage) {
+   dst_clip = plane_state->dst;
  
-	if (!drm_dev_enter(drm, &idx))

-   return;
+   if (!drm_rect_intersect(&dst_clip, &damage))
+   continue;
  
-	ssd130x_fb_blit_rect(plane_state->fb, &shadow_plane_state->data[0], &dst_clip);

+   ssd130x_fb_blit_rect(plane_state->fb, &shadow_plane_state->data[0], 
&dst_clip);
+   }
  
  	drm_dev_exit(idx);

  }


--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev


OpenPGP_signature
Description: OpenPGP digital signature

Re: linux-next: build failure after merge of the drm tree

2022-10-05 Thread Hamza Mahfooz


On 2022-10-05 11:30, Alex Deucher wrote:

@Mahfooz, Hamza
@Aurabindo Pillai can you get this fixed up?



Seems like this is a false positive for GCC versions pre-12, because I'm 
not seeing this warning on GCC 12.2.

However, we can fix this for older GCC versions with the following:

diff --git a/drivers/gpu/drm/amd/display/dc/dc_stream.h 
b/drivers/gpu/drm/amd/display/dc/dc_stream.h

index 9e6025c98db9..67fede4bf248 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_stream.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_stream.h
@@ -238,7 +238,7 @@ struct dc_stream_state {

/* writeback */
unsigned int num_wb_info;
-   struct dc_writeback_info writeback_info[MAX_DWB_PIPES];
+   struct dc_writeback_info writeback_info[MAX_DWB_PIPES + 1];
const struct dc_transfer_func *func_shaper;
const struct dc_3dlut *lut3d_func;
/* Computed state bits */



Thanks,

Alex

On Tue, Oct 4, 2022 at 7:39 AM Mark Brown  wrote:


On Tue, Oct 04, 2022 at 02:05:58PM +1100, Stephen Rothwell wrote:

On Tue, 4 Oct 2022 12:24:37 +1000 David Airlie  wrote:

On Tue, Oct 4, 2022 at 12:21 PM Stephen Rothwell  wrote:



I'm not seeing it here, what gcc is this with?



I am using an x86_64 cross compiler hosted on ppc64le - gcc v11.2.0 (on
Debian).


I was seeing this with an x86_64 cross compiler hosted on arm64 -
Ubuntu 11.2.0-17ubuntu1 from the looks of it.


--
Hamza

Re: [PATCH v4] drm/ast: Add Atomic gamma lut support for aspeed

2022-10-05 Thread Jocelyn Falempe


On 30/09/2022 12:45, Thomas Zimmermann wrote:

Hi,

looks good to me. Let's wait until next week before landing the patch, 
so that others can comment on it.


applied to drm-misc-next

Thanks,

--

Jocelyn



Best regards
Thomas

Am 30.09.22 um 11:47 schrieb Jocelyn Falempe:

The current ast driver only supports legacy gamma interface.
This also fixes a Gnome3/Wayland error which incorrectly adds
gamma to atomic commit:
"Page flip discarded: CRTC property (GAMMA_LUT) not found"

I only tested remotely, so I wasn't able to check that it had
an effect on the VGA output. But when activating "Night Light"
in Gnome, ast_crtc_load_lut() is called.

v2: use the same functions as mgag200.
 handle 16bits color mode.

v3: Check gamma_lut size in atomic check.

v4: revert 16bits mode, v1 was correct.
 make sure gamma table are set when primary plane format
 changes.
 remove rgb888 format that is not used.

Signed-off-by: Jocelyn Falempe 
Tested-by: Thomas Zimmermann 
Reviewed-by: Thomas Zimmermann 
---
  drivers/gpu/drm/ast/ast_mode.c | 87 +++---
  1 file changed, 70 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c 
b/drivers/gpu/drm/ast/ast_mode.c

index 214b10178454..89fcb8e3ea16 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -49,6 +49,8 @@
  #include "ast_drv.h"
  #include "ast_tables.h"
+#define AST_LUT_SIZE 256
+
  static inline void ast_load_palette_index(struct ast_private *ast,
   u8 index, u8 red, u8 green,
   u8 blue)
@@ -63,20 +65,46 @@ static inline void ast_load_palette_index(struct 
ast_private *ast,

  ast_io_read8(ast, AST_IO_SEQ_PORT);
  }
-static void ast_crtc_load_lut(struct ast_private *ast, struct 
drm_crtc *crtc)

+static void ast_crtc_set_gamma_linear(struct ast_private *ast,
+  const struct drm_format_info *format)
  {
-    u16 *r, *g, *b;
  int i;
-    if (!crtc->enabled)
-    return;
+    switch (format->format) {
+    case DRM_FORMAT_C8: /* In this case, gamma table is used as color 
palette */

+    case DRM_FORMAT_RGB565:
+    case DRM_FORMAT_XRGB:
+    for (i = 0; i < AST_LUT_SIZE; i++)
+    ast_load_palette_index(ast, i, i, i, i);
+    break;
+    default:
+    drm_warn_once(&ast->base, "Unsupported format %p4cc for gamma 
correction\n",

+  &format->format);
+    break;
+    }
+}
-    r = crtc->gamma_store;
-    g = r + crtc->gamma_size;
-    b = g + crtc->gamma_size;
+static void ast_crtc_set_gamma(struct ast_private *ast,
+   const struct drm_format_info *format,
+   struct drm_color_lut *lut)
+{
+    int i;
-    for (i = 0; i < 256; i++)
-    ast_load_palette_index(ast, i, *r++ >> 8, *g++ >> 8, *b++ >> 8);
+    switch (format->format) {
+    case DRM_FORMAT_C8: /* In this case, gamma table is used as color 
palette */

+    case DRM_FORMAT_RGB565:
+    case DRM_FORMAT_XRGB:
+    for (i = 0; i < AST_LUT_SIZE; i++)
+    ast_load_palette_index(ast, i,
+   lut[i].red >> 8,
+   lut[i].green >> 8,
+   lut[i].blue >> 8);
+    break;
+    default:
+    drm_warn_once(&ast->base, "Unsupported format %p4cc for gamma 
correction\n",

+  &format->format);
+    break;
+    }
  }
  static bool ast_get_vbios_mode_info(const struct drm_format_info 
*format,
@@ -1018,9 +1046,11 @@ static void ast_crtc_dpms(struct drm_crtc 
*crtc, int mode)

  ast_set_color_reg(ast, format);
  ast_set_vbios_color_reg(ast, format, vbios_mode_info);
+    if (crtc->state->gamma_lut)
+    ast_crtc_set_gamma(ast, format, 
crtc->state->gamma_lut->data);

+    else
+    ast_crtc_set_gamma_linear(ast, format);
  }
-
-    ast_crtc_load_lut(ast, crtc);
  break;
  case DRM_MODE_DPMS_STANDBY:
  case DRM_MODE_DPMS_SUSPEND:
@@ -1109,6 +1139,8 @@ static int ast_crtc_helper_atomic_check(struct 
drm_crtc *crtc,

  struct drm_atomic_state *state)
  {
  struct drm_crtc_state *crtc_state = 
drm_atomic_get_new_crtc_state(state, crtc);
+    struct drm_crtc_state *old_crtc_state = 
drm_atomic_get_old_crtc_state(state, crtc);
+    struct ast_crtc_state *old_ast_crtc_state = 
to_ast_crtc_state(old_crtc_state);

  struct drm_device *dev = crtc->dev;
  struct ast_crtc_state *ast_state;
  const struct drm_format_info *format;
@@ -1128,6 +1160,22 @@ static int ast_crtc_helper_atomic_check(struct 
drm_crtc *crtc,

  if (drm_WARN_ON_ONCE(dev, !format))
  return -EINVAL; /* BUG: We didn't set format in primary 
check(). */

+    /*
+ * The gamma LUT has to be reloaded after changing the primary
+ * plane's color format.
+ */
+    if (old_ast_crtc_state->format != format)
+    crtc_state->color_mgmt_changed = true;
+
+    if (

[PATCH v3 2/2] drm/i915/slpc: Update the frequency debugfs

2022-10-05 Thread Vinay Belgaumkar

Read the values stored in the SLPC structures. Remove the
fields that are no longer valid (like RPS interrupts) as
well.

v2: Move all functionality changes to this patch (Jani)
v3: Fix compile warning and if condition (Jani)

Signed-off-by: Vinay Belgaumkar 
---
 drivers/gpu/drm/i915/gt/intel_rps.c | 46 -
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c 
b/drivers/gpu/drm/i915/gt/intel_rps.c
index 737db780db00..fc23c562d9b2 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.c
+++ b/drivers/gpu/drm/i915/gt/intel_rps.c
@@ -2219,7 +2219,7 @@ u32 intel_rps_get_rpn_frequency(struct intel_rps *rps)
return intel_gpu_freq(rps, rps->min_freq);
 }
 
-void gen6_rps_frequency_dump(struct intel_rps *rps, struct drm_printer *p)
+static void rps_frequency_dump(struct intel_rps *rps, struct drm_printer *p)
 {
struct intel_gt *gt = rps_to_gt(rps);
struct drm_i915_private *i915 = gt->i915;
@@ -2382,6 +2382,50 @@ void gen6_rps_frequency_dump(struct intel_rps *rps, 
struct drm_printer *p)
   intel_gpu_freq(rps, rps->efficient_freq));
 }
 
+static void slpc_frequency_dump(struct intel_rps *rps, struct drm_printer *p)
+{
+   struct intel_gt *gt = rps_to_gt(rps);
+   struct intel_uncore *uncore = gt->uncore;
+   struct intel_rps_freq_caps caps;
+   u32 pm_mask;
+
+   gen6_rps_get_freq_caps(rps, &caps);
+   pm_mask = intel_uncore_read(uncore, GEN6_PMINTRMSK);
+
+   drm_printf(p, "PM MASK=0x%08x\n", pm_mask);
+   drm_printf(p, "pm_intrmsk_mbz: 0x%08x\n",
+  rps->pm_intrmsk_mbz);
+   drm_printf(p, "RPSTAT1: 0x%08x\n", intel_uncore_read(uncore, 
GEN6_RPSTAT1));
+   drm_printf(p, "RPNSWREQ: %dMHz\n", 
intel_rps_get_requested_frequency(rps));
+   drm_printf(p, "Lowest (RPN) frequency: %dMHz\n",
+  intel_gpu_freq(rps, caps.min_freq));
+   drm_printf(p, "Nominal (RP1) frequency: %dMHz\n",
+  intel_gpu_freq(rps, caps.rp1_freq));
+   drm_printf(p, "Max non-overclocked (RP0) frequency: %dMHz\n",
+  intel_gpu_freq(rps, caps.rp0_freq));
+   drm_printf(p, "Current freq: %d MHz\n",
+  intel_rps_get_requested_frequency(rps));
+   drm_printf(p, "Actual freq: %d MHz\n",
+  intel_rps_read_actual_frequency(rps));
+   drm_printf(p, "Min freq: %d MHz\n",
+  intel_rps_get_min_frequency(rps));
+   drm_printf(p, "Boost freq: %d MHz\n",
+  intel_rps_get_boost_frequency(rps));
+   drm_printf(p, "Max freq: %d MHz\n",
+  intel_rps_get_max_frequency(rps));
+   drm_printf(p,
+  "efficient (RPe) frequency: %d MHz\n",
+  intel_gpu_freq(rps, caps.rp1_freq));
+}
+
+void gen6_rps_frequency_dump(struct intel_rps *rps, struct drm_printer *p)
+{
+   if (rps_uses_slpc(rps))
+   return slpc_frequency_dump(rps, p);
+   else
+   return rps_frequency_dump(rps, p);
+}
+
 static int set_max_freq(struct intel_rps *rps, u32 val)
 {
struct drm_i915_private *i915 = rps_to_i915(rps);
-- 
2.35.1

[PATCH v3 1/2] drm/i915: Add a wrapper for frequency debugfs

2022-10-05 Thread Vinay Belgaumkar

Move it to the RPS source file.

v2: Separate out code movement and functional changes (Jani)

Signed-off-by: Vinay Belgaumkar 
---
 drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 157 +
 drivers/gpu/drm/i915/gt/intel_rps.c   | 163 ++
 drivers/gpu/drm/i915/gt/intel_rps.h   |   3 +
 3 files changed, 167 insertions(+), 156 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c 
b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
index 10f680dbd7b6..40d0a3be42ac 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
@@ -344,162 +344,7 @@ void intel_gt_pm_frequency_dump(struct intel_gt *gt, 
struct drm_printer *p)
drm_printf(p, "efficient (RPe) frequency: %d MHz\n",
   intel_gpu_freq(rps, rps->efficient_freq));
} else if (GRAPHICS_VER(i915) >= 6) {
-   u32 rp_state_limits;
-   u32 gt_perf_status;
-   struct intel_rps_freq_caps caps;
-   u32 rpmodectl, rpinclimit, rpdeclimit;
-   u32 rpstat, cagf, reqf;
-   u32 rpcurupei, rpcurup, rpprevup;
-   u32 rpcurdownei, rpcurdown, rpprevdown;
-   u32 rpupei, rpupt, rpdownei, rpdownt;
-   u32 pm_ier, pm_imr, pm_isr, pm_iir, pm_mask;
-
-   rp_state_limits = intel_uncore_read(uncore, 
GEN6_RP_STATE_LIMITS);
-   gen6_rps_get_freq_caps(rps, &caps);
-   if (IS_GEN9_LP(i915))
-   gt_perf_status = intel_uncore_read(uncore, 
BXT_GT_PERF_STATUS);
-   else
-   gt_perf_status = intel_uncore_read(uncore, 
GEN6_GT_PERF_STATUS);
-
-   /* RPSTAT1 is in the GT power well */
-   intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL);
-
-   reqf = intel_uncore_read(uncore, GEN6_RPNSWREQ);
-   if (GRAPHICS_VER(i915) >= 9) {
-   reqf >>= 23;
-   } else {
-   reqf &= ~GEN6_TURBO_DISABLE;
-   if (IS_HASWELL(i915) || IS_BROADWELL(i915))
-   reqf >>= 24;
-   else
-   reqf >>= 25;
-   }
-   reqf = intel_gpu_freq(rps, reqf);
-
-   rpmodectl = intel_uncore_read(uncore, GEN6_RP_CONTROL);
-   rpinclimit = intel_uncore_read(uncore, GEN6_RP_UP_THRESHOLD);
-   rpdeclimit = intel_uncore_read(uncore, GEN6_RP_DOWN_THRESHOLD);
-
-   rpstat = intel_uncore_read(uncore, GEN6_RPSTAT1);
-   rpcurupei = intel_uncore_read(uncore, GEN6_RP_CUR_UP_EI) & 
GEN6_CURICONT_MASK;
-   rpcurup = intel_uncore_read(uncore, GEN6_RP_CUR_UP) & 
GEN6_CURBSYTAVG_MASK;
-   rpprevup = intel_uncore_read(uncore, GEN6_RP_PREV_UP) & 
GEN6_CURBSYTAVG_MASK;
-   rpcurdownei = intel_uncore_read(uncore, GEN6_RP_CUR_DOWN_EI) & 
GEN6_CURIAVG_MASK;
-   rpcurdown = intel_uncore_read(uncore, GEN6_RP_CUR_DOWN) & 
GEN6_CURBSYTAVG_MASK;
-   rpprevdown = intel_uncore_read(uncore, GEN6_RP_PREV_DOWN) & 
GEN6_CURBSYTAVG_MASK;
-
-   rpupei = intel_uncore_read(uncore, GEN6_RP_UP_EI);
-   rpupt = intel_uncore_read(uncore, GEN6_RP_UP_THRESHOLD);
-
-   rpdownei = intel_uncore_read(uncore, GEN6_RP_DOWN_EI);
-   rpdownt = intel_uncore_read(uncore, GEN6_RP_DOWN_THRESHOLD);
-
-   cagf = intel_rps_read_actual_frequency(rps);
-
-   intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL);
-
-   if (GRAPHICS_VER(i915) >= 11) {
-   pm_ier = intel_uncore_read(uncore, 
GEN11_GPM_WGBOXPERF_INTR_ENABLE);
-   pm_imr = intel_uncore_read(uncore, 
GEN11_GPM_WGBOXPERF_INTR_MASK);
-   /*
-* The equivalent to the PM ISR & IIR cannot be read
-* without affecting the current state of the system
-*/
-   pm_isr = 0;
-   pm_iir = 0;
-   } else if (GRAPHICS_VER(i915) >= 8) {
-   pm_ier = intel_uncore_read(uncore, GEN8_GT_IER(2));
-   pm_imr = intel_uncore_read(uncore, GEN8_GT_IMR(2));
-   pm_isr = intel_uncore_read(uncore, GEN8_GT_ISR(2));
-   pm_iir = intel_uncore_read(uncore, GEN8_GT_IIR(2));
-   } else {
-   pm_ier = intel_uncore_read(uncore, GEN6_PMIER);
-   pm_imr = intel_uncore_read(uncore, GEN6_PMIMR);
-   pm_isr = intel_uncore_read(uncore, GEN6_PMISR);
-   pm_iir = intel_uncore_read(uncore, GEN6_PMIIR);
-   }
-   pm_mask = intel_uncore_read(uncore, GEN6_PMINTRMSK);
-
-   drm_printf(p, "Video Turbo Mode: %s\n",
-

[PATCH v3 0/2] drm/i915/slpc: Update frequency debugfs for SLPC

2022-10-05 Thread Vinay Belgaumkar

Remove the RPS related information that is not valid when
SLPC is enabled.

v2: Add version numbers and address other comments (Jani)
v3: Fix compile warning

Signed-off-by: Vinay Belgaumkar 

Vinay Belgaumkar (2):
  drm/i915: Add a wrapper for frequency debugfs
  drm/i915/slpc: Update the frequency debugfs

 drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 157 +
 drivers/gpu/drm/i915/gt/intel_rps.c   | 207 ++
 drivers/gpu/drm/i915/gt/intel_rps.h   |   3 +
 3 files changed, 211 insertions(+), 156 deletions(-)

-- 
2.35.1

Re: [PATCH 5/5] drm/dsc: Prevent negative BPG offsets from shadowing adjacent bitfields

On 10/4/2022 3:39 PM, Marijn Suijten wrote:

On 2022-10-04 15:31:10, Abhinav Kumar wrote:

On 10/4/2022 2:57 PM, Marijn Suijten wrote:

[..]
Alas, as explained in the cover letter I opted to perform the masking in
the PPS packing code as the DSC block code also reads these values, and
would suddenly write 6-bit intead of 8-bit values to the
DSC_RANGE_BPG_OFFSET registers. Quick testing on the mentioned sdm845
platform shows no regressions, but I'm not sure if that's safe to rely
on?

I looked up the MDP_DSC_0_RANGE_BPG_OFFSET_* registers.
They take only a 6-bit value according to the SW documentation ( bits 5:0 )

It was always expecting only a 6-bit value and not 8.

So this change is safe.

Ack, I think that implies I should make this change and move the masks
to the DSI driver?

hmm downstream seems to have the & just before the reg write

https://git.codelinaro.org/clo/la/platform/vendor/opensource/display-drivers/-/blob/DISPLAY.LA.2.0.r1-08000-WAIPIO.0/msm/sde/sde_hw_dsc_1_2.c#L310

But yes, we can move this to the dsi calculation to match what others
are doing. I am fine with that.

If you want to move to helper, other drivers need to be changed too to
remove duplicate & 0x3f.

Sure, we only have to confirm whether those drivers also read back the
value(s) in rc_range_params, and expect / allow this to be 8 instead of
6 bits.

FWIW, this too has already been fixed in the latest downstream driver too.

What is this supposed to mean? Is there a downstream DPU project that
has pending patches needing to be upstreamed? Or is the downstream SDE,
techpack/display, or whatever it is called nowadays, slowly using more
DRM structs like drm_dsc_config and this drm_dsc_pps_payload_pack()
helper function as pointed out in an earlier mail?

No, what I meant was, the version of downstream driver based on which
the upstream DSC was made seems to be an older version. Downstream
drivers keep getting updated and we always keep trying to align with
upstream structs.

This is true not just for DSC but even other blocks.

So as part of that effort, we started using struct drm_dsc_config . That
change was made on newer chipsets. But the downstream SW on sdm845 based
on which the DSC was upstreamed seems like didnt have that. Hence all
this redundant math happened.

So this comment was more of a explanation about why this issue happened
even though latest downstream didnt have this issue.

Thanks, I understood most of that but wasn't aware these exact "issues"
were also addressed downstream (by i.e. also using the upstream
structs).

Even I wasnt. When I was reviewing this series, it seemed like a valid
change so I checked the latest downstream code like i always do to see
whether and how this is handled and found that these issues were
addressed there so thought i would update that here.

Offtopic: are SDE and DPU growing closer together, hopefully achieving
feature parity allowing the SDE project to be dropped in favour of a
fully upstreamed DPU driver for day-one out-of-the-box mainline support
for new SoCs (as long as work is published and on its way upstream)?

There is still a lot of gap between SDE and DPU drivers at this point.
We keep trying to upstream as many features as possible to minimize the
gap but there is still a lot of work to do.

Glad to hear, but that sounds like a very hard to close gap unless
downstream "just works on DPU" instead of having parallel development on
two "competing" drivers for the exact same hardware.

Its not really parallel development OR competing drivers.
The design of these features downstream (not just DSC but many others)
is quite different because some of the downstream designs wont get
accepted upstream as its tightly coupled with our
compositor/device-tree. Thats where we are slowly trying to implement
these in an upstream compatible way.

BTW, that being said. Its not always the case that every issue seen
upstream has already been fixed downstream. In fact, on some occasions
we found something fixed in upstream in a better way and ported them
downstream too.

Thanks

Abhinav

- Marijn

Re: linux-next: build failure after merge of the drm tree

2022-10-05 Thread Alex Deucher

@Mahfooz, Hamza
@Aurabindo Pillai can you get this fixed up?

Thanks,

Alex

On Tue, Oct 4, 2022 at 7:39 AM Mark Brown  wrote:
>
> On Tue, Oct 04, 2022 at 02:05:58PM +1100, Stephen Rothwell wrote:
> > On Tue, 4 Oct 2022 12:24:37 +1000 David Airlie  wrote:
> > > On Tue, Oct 4, 2022 at 12:21 PM Stephen Rothwell  
> > > wrote:
>
> > > I'm not seeing it here, what gcc is this with?
>
> > I am using an x86_64 cross compiler hosted on ppc64le - gcc v11.2.0 (on
> > Debian).
>
> I was seeing this with an x86_64 cross compiler hosted on arm64 -
> Ubuntu 11.2.0-17ubuntu1 from the looks of it.

Re: [PATCH 10/16] drm/udl: Use damage iterator


Hi

Am 05.10.22 um 00:28 schrieb Javier Martinez Canillas:

On 9/19/22 15:04, Thomas Zimmermann wrote:

Use a damage iterator to process damage areas individually. Merging
damage areas can resul tin large updates of unchanged framebuffer


result in


regions. As USB is rather slow, it's better to process damage areas
individually and hence minimize USB-transfered data.

Signed-off-by: Thomas Zimmermann 
---


Reviewed-by: Javier Martinez Canillas 

but I've a comment below.

  
  /*

@@ -301,16 +291,26 @@ static void udl_primary_plane_helper_atomic_update(struct 
drm_plane *plane,
struct drm_shadow_plane_state *shadow_plane_state = 
to_drm_shadow_plane_state(plane_state);
struct drm_framebuffer *fb = plane_state->fb;
struct drm_plane_state *old_plane_state = 
drm_atomic_get_old_plane_state(state, plane);
-   struct drm_rect rect;
-   int idx;
+   struct drm_atomic_helper_damage_iter iter;
+   struct drm_rect damage;
+   int ret, idx;
  
-	if (!drm_dev_enter(dev, &idx))

+   ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
+   if (ret)
return;



This is an unrelated change. The sync was made in udl_handle_damage() before
and you are moving to udl_primary_plane_helper_atomic_update() in this patch.

I don't have a strong opinion and I see that there are drivers that do once
before iterating over the damage areas and others do the sync for each damage
area update. But it would be good to mention that this change is done too and
provided some justification.


OK, I'll do that. Very briefly; it's for minimizing the overheads of the 
possible locking and synchronization, and to maybe it keeps possible 
writers out while we copy from the buffer.


Best regards
Thomas





--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH v2] drm/bridge: ps8640: Add software to support aux defer

2022-10-05 Thread Doug Anderson

Hi,

On Fri, Sep 30, 2022 at 7:20 AM Doug Anderson  wrote:
>
> Hi,
>
> On Thu, Sep 29, 2022 at 9:25 PM Jason Yen
>  wrote:
> >
> > This chip can not handle aux defer if the host directly program
> > its aux registers to access edid/dpcd. So we need let software
> > to handle the aux defer situation.
> >
> > Signed-off-by: Jason Yen 
> > ---
> >
> > Changes in v2:
> > - Add aux defer handler
> > - Remove incorrect statements
> >
> >  drivers/gpu/drm/bridge/parade-ps8640.c | 9 -
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c 
> > b/drivers/gpu/drm/bridge/parade-ps8640.c
> > index 31e88cb39f8a..76ada237096d 100644
> > --- a/drivers/gpu/drm/bridge/parade-ps8640.c
> > +++ b/drivers/gpu/drm/bridge/parade-ps8640.c
> > @@ -286,7 +286,6 @@ static ssize_t ps8640_aux_transfer_msg(struct 
> > drm_dp_aux *aux,
> > }
> >
> > switch (data & SWAUX_STATUS_MASK) {
> > -   /* Ignore the DEFER cases as they are already handled in hardware */
> > case SWAUX_STATUS_NACK:
> > case SWAUX_STATUS_I2C_NACK:
> > /*
> > @@ -303,6 +302,14 @@ static ssize_t ps8640_aux_transfer_msg(struct 
> > drm_dp_aux *aux,
> > case SWAUX_STATUS_ACKM:
> > len = data & SWAUX_M_MASK;
> > break;
> > +   case SWAUX_STATUS_DEFER:
> > +   case SWAUX_STATUS_I2C_DEFER:
> > +   if (is_native_aux)
> > +   msg->reply |= DP_AUX_NATIVE_REPLY_DEFER;
> > +   else
> > +   msg->reply |= DP_AUX_I2C_REPLY_DEFER;
> > +   len = data & SWAUX_M_MASK;
> > +   break;
>
> This seems fine to me now. There is nothing too controversial here but
> I'll still give this a few days on the list for anyone else to speak
> up. I'll plan to land it midway through next week unless anything
> comes up.
>
> Reviewed-by: Douglas Anderson 

As promised, pushed to drm-misc-next:

562d2dd87028 drm/bridge: ps8640: Add software to support aux defer

[PATCH v7 05/10] drm: bridge: samsung-dsim: Add atomic_check

Look like an explicit fixing up of mode_flags is required for DSIM IP
present in i.MX8M Mini/Nano SoCs.

At least the LCDIF + DSIM needs active low sync polarities in order
to correlate the correct sync flags of the surrounding components in
the chain to make sure the whole pipeline can work properly.

On the other hand the i.MX 8M Mini Applications Processor Reference Manual,
Rev. 3, 11/2020 says.
"13.6.3.5.2 RGB interface
 Vsync, Hsync, and VDEN are active high signals."

No clear evidence about whether it can be documentation issues or
something, so added a comment FIXME for this and updated the active low
sync polarities using SAMSUNG_DSIM_TYPE_IMX8MM hw_type.

v7:
* fix the hw_type checking logic

v6:
* none

v5:
* rebase based new bridge changes [mszyprow]
* remove DSIM_QUIRK_FIXUP_SYNC_POL
* add hw_type check for sync polarities change.

v4:
* none

v3:
* add DSIM_QUIRK_FIXUP_SYNC_POL to handle mode_flasg fixup

v2:
* none

v1:
* fix mode flags in atomic_check instead of mode_fixup

Signed-off-by: Jagan Teki 
---
 drivers/gpu/drm/bridge/samsung-dsim.c | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
b/drivers/gpu/drm/bridge/samsung-dsim.c
index 563781bb27c4..b46346232c52 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1315,6 +1315,31 @@ static void samsung_dsim_atomic_post_disable(struct 
drm_bridge *bridge,
pm_runtime_put_sync(dsi->dev);
 }
 
+static int samsung_dsim_atomic_check(struct drm_bridge *bridge,
+struct drm_bridge_state *bridge_state,
+struct drm_crtc_state *crtc_state,
+struct drm_connector_state *conn_state)
+{
+   struct samsung_dsim *dsi = bridge_to_dsi(bridge);
+   struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode;
+
+   if (dsi->plat_data->hw_type == SAMSUNG_DSIM_TYPE_IMX8MM) {
+   /**
+* FIXME:
+* At least LCDIF + DSIM needs active low sync,
+* but i.MX 8M Mini Applications Processor Reference Manual,
+* Rev. 3, 11/2020 says
+*
+* 13.6.3.5.2 RGB interface
+* Vsync, Hsync, and VDEN are active high signals.
+*/
+   adjusted_mode->flags |= (DRM_MODE_FLAG_NHSYNC | 
DRM_MODE_FLAG_NVSYNC);
+   adjusted_mode->flags &= ~(DRM_MODE_FLAG_PHSYNC | 
DRM_MODE_FLAG_PVSYNC);
+   }
+
+   return 0;
+}
+
 static void samsung_dsim_mode_set(struct drm_bridge *bridge,
  const struct drm_display_mode *mode,
  const struct drm_display_mode *adjusted_mode)
@@ -1353,6 +1378,7 @@ static const struct drm_bridge_funcs 
samsung_dsim_bridge_funcs = {
.atomic_duplicate_state = 
drm_atomic_helper_bridge_duplicate_state,
.atomic_destroy_state   = 
drm_atomic_helper_bridge_destroy_state,
.atomic_reset   = drm_atomic_helper_bridge_reset,
+   .atomic_check   = samsung_dsim_atomic_check,
.atomic_pre_enable  = samsung_dsim_atomic_pre_enable,
.atomic_enable  = samsung_dsim_atomic_enable,
.atomic_disable = samsung_dsim_atomic_disable,
-- 
2.25.1

[PATCH v7 10/10] drm: bridge: samsung-dsim: Add i.MX8MM support

Samsung MIPI DSIM master can also be found in i.MX8MM SoC.

Add compatible and associated driver_data for it.

v7, v6:
* none

v3:
* enable DSIM_QUIRK_FIXUP_SYNC_POL quirk

v5:
* [mszyprow] rebased and adjusted to the new driver initialization
* drop quirk

v4:
* none

v3:
* enable DSIM_QUIRK_FIXUP_SYNC_POL quirk

v2:
* collect Laurent r-b

v1:
* none

Reviewed-by: Laurent Pinchart 
Signed-off-by: Marek Szyprowski 
Signed-off-by: Jagan Teki 
---
 drivers/gpu/drm/bridge/samsung-dsim.c | 45 +++
 1 file changed, 45 insertions(+)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
b/drivers/gpu/drm/bridge/samsung-dsim.c
index f5cd80641cea..1b5ba33dc635 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -360,6 +360,24 @@ static const unsigned int exynos5433_reg_values[] = {
[PHYTIMING_HS_TRAIL] = DSIM_PHYTIMING2_HS_TRAIL(0x0c),
 };
 
+static const unsigned int imx8mm_dsim_reg_values[] = {
+   [RESET_TYPE] = DSIM_SWRST,
+   [PLL_TIMER] = 500,
+   [STOP_STATE_CNT] = 0xf,
+   [PHYCTRL_ULPS_EXIT] = 0,
+   [PHYCTRL_VREG_LP] = 0,
+   [PHYCTRL_SLEW_UP] = 0,
+   [PHYTIMING_LPX] = DSIM_PHYTIMING_LPX(0x06),
+   [PHYTIMING_HS_EXIT] = DSIM_PHYTIMING_HS_EXIT(0x0b),
+   [PHYTIMING_CLK_PREPARE] = DSIM_PHYTIMING1_CLK_PREPARE(0x07),
+   [PHYTIMING_CLK_ZERO] = DSIM_PHYTIMING1_CLK_ZERO(0x26),
+   [PHYTIMING_CLK_POST] = DSIM_PHYTIMING1_CLK_POST(0x0d),
+   [PHYTIMING_CLK_TRAIL] = DSIM_PHYTIMING1_CLK_TRAIL(0x08),
+   [PHYTIMING_HS_PREPARE] = DSIM_PHYTIMING2_HS_PREPARE(0x08),
+   [PHYTIMING_HS_ZERO] = DSIM_PHYTIMING2_HS_ZERO(0x0d),
+   [PHYTIMING_HS_TRAIL] = DSIM_PHYTIMING2_HS_TRAIL(0x0b),
+};
+
 static const struct samsung_dsim_driver_data exynos3_dsi_driver_data = {
.reg_ofs = exynos_reg_ofs,
.plltmr_reg = 0x50,
@@ -421,6 +439,23 @@ static const struct samsung_dsim_driver_data 
exynos5422_dsi_driver_data = {
.reg_values = exynos5422_reg_values,
 };
 
+static const struct samsung_dsim_driver_data imx8mm_dsi_driver_data = {
+   .reg_ofs = exynos5433_reg_ofs,
+   .plltmr_reg = 0xa0,
+   .has_clklane_stop = 1,
+   .num_clks = 2,
+   .max_freq = 2100,
+   .wait_for_reset = 0,
+   .num_bits_resol = 12,
+   /**
+* FIXME:
+* Offset value used from downstream drivers/gpu/drm/bridge/sec-dsim.c
+* remove this comment if it is true else update the logic.
+*/
+   .pll_p_offset = 14,
+   .reg_values = imx8mm_dsim_reg_values,
+};
+
 static const struct samsung_dsim_driver_data *
 samsung_dsim_types[SAMSUNG_DSIM_TYPE_COUNT] = {
[SAMSUNG_DSIM_TYPE_EXYNOS3250] = &exynos3_dsi_driver_data,
@@ -428,6 +463,7 @@ samsung_dsim_types[SAMSUNG_DSIM_TYPE_COUNT] = {
[SAMSUNG_DSIM_TYPE_EXYNOS5410] = &exynos5_dsi_driver_data,
[SAMSUNG_DSIM_TYPE_EXYNOS5422] = &exynos5422_dsi_driver_data,
[SAMSUNG_DSIM_TYPE_EXYNOS5433] = &exynos5433_dsi_driver_data,
+   [SAMSUNG_DSIM_TYPE_IMX8MM] = &imx8mm_dsi_driver_data,
 };
 
 static inline struct samsung_dsim *host_to_dsi(struct mipi_dsi_host *h)
@@ -1788,7 +1824,16 @@ const struct dev_pm_ops samsung_dsim_pm_ops = {
 };
 EXPORT_SYMBOL_GPL(samsung_dsim_pm_ops);
 
+static const struct samsung_dsim_plat_data samsung_dsim_imx8mm_pdata = {
+   .hw_type = SAMSUNG_DSIM_TYPE_IMX8MM,
+   .host_ops = &samsung_dsim_generic_host_ops,
+};
+
 static const struct of_device_id samsung_dsim_of_match[] = {
+   {
+   .compatible = "fsl,imx8mm-mipi-dsim",
+   .data = &samsung_dsim_imx8mm_pdata,
+   },
{ /* sentinel. */ }
 };
 MODULE_DEVICE_TABLE(of, samsung_dsim_of_match);
-- 
2.25.1

[PATCH v7 08/10] drm: bridge: samsung-dsim: Add input_bus_flags

eLCDIF is expecting to have input_bus_flags as DE_LOW in order to
set active low during valid data transfer on each horizontal line.

Add DE_LOW flag via drm bridge timings.

v7, v6:
* none

v5:
* rebased based on updated bridge changes

v4, v3, v2, v1:
* none

Signed-off-by: Jagan Teki 
---
 drivers/gpu/drm/bridge/samsung-dsim.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
b/drivers/gpu/drm/bridge/samsung-dsim.c
index f714e49c1eab..f5cd80641cea 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1601,6 +1601,10 @@ static const struct samsung_dsim_host_ops 
samsung_dsim_generic_host_ops = {
.unregister_host = samsung_dsim_unregister_host,
 };
 
+static const struct drm_bridge_timings samsung_dsim_bridge_timings = {
+   .input_bus_flags = DRM_BUS_FLAG_DE_LOW,
+};
+
 int samsung_dsim_probe(struct platform_device *pdev)
 {
struct device *dev = &pdev->dev;
@@ -1681,6 +1685,7 @@ int samsung_dsim_probe(struct platform_device *pdev)
 
dsi->bridge.funcs = &samsung_dsim_bridge_funcs;
dsi->bridge.of_node = dev->of_node;
+   dsi->bridge.timings = &samsung_dsim_bridge_timings;
dsi->bridge.type = DRM_MODE_CONNECTOR_DSI;
 
if (dsi->plat_data->host_ops && dsi->plat_data->host_ops->register_host)
-- 
2.25.1

[PATCH v7 09/10] dt-bindings: display: exynos: dsim: Add NXP i.MX8MM support

Samsung MIPI DSIM bridge can also be found in i.MX8MM SoC.

Add dt-bingings for it.

v7, v6, v5, v4:
* none

v3:
* collect Rob Acked-by

v2:
* updated comments

v1:
* new patch

Acked-by: Rob Herring 
Signed-off-by: Jagan Teki 
---
 Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt 
b/Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt
index be377786e8cd..8efcf4728e0b 100644
--- a/Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt
+++ b/Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt
@@ -7,6 +7,7 @@ Required properties:
"samsung,exynos5410-mipi-dsi" /* for Exynos5410/5420/5440 SoCs 
*/
"samsung,exynos5422-mipi-dsi" /* for Exynos5422/5800 SoCs */
"samsung,exynos5433-mipi-dsi" /* for Exynos5433 SoCs */
+   "fsl,imx8mm-mipi-dsim" /* for i.MX8M Mini SoCs */
   - reg: physical base address and length of the registers set for the device
   - interrupts: should contain DSI interrupt
   - clocks: list of clock specifiers, must contain an entry for each required
-- 
2.25.1

[PATCH v7 06/10] drm: bridge: samsung-dsim: Add platform PLL_P (PMS_P) offset

Look like PLL PMS_P offset value varies between platforms that have
Samsung DSIM IP.

However, there is no clear evidence for it as both Exynos and i.MX
8M Mini Application Processor Reference Manual is still referring
the PMS_P offset as 13.

The offset 13 is not working for i.MX8M Mini SoCs but the downstream
NXP sec-dsim.c driver is using offset 14 for i.MX8M Mini SoC platforms
[1] [2].

PMS_P value set in sec_mipi_dsim_check_pll_out using PLLCTRL_SET_P()
with offset 13 and then an additional offset of one bit added in
sec_mipi_dsim_config_pll via PLLCTRL_SET_PMS().

Not sure whether it is reference manual documentation or something else
but this patch trusts the downstream code and handle PLL_P offset via
platform driver data so-that imx8mm driver data shall use pll_p_offset
to 14.

[1] 
https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/gpu/drm/bridge/sec-dsim.c?h=imx_5.4.47_2.2.0#n210
[2] 
https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/gpu/drm/bridge/sec-dsim.c?h=imx_5.4.47_2.2.0#n211

v7, v6:
* none

v5:
* updated clear commit message

v4, v3, v2:
* none

v1:
* updated commit message
* add downstream driver link

Signed-off-by: Frieder Schrempf 
Signed-off-by: Jagan Teki 
---
 drivers/gpu/drm/bridge/samsung-dsim.c | 10 --
 include/drm/bridge/samsung-dsim.h |  1 +
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
b/drivers/gpu/drm/bridge/samsung-dsim.c
index b46346232c52..41970e794a7c 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -168,7 +168,7 @@
 /* DSIM_PLLCTRL */
 #define DSIM_FREQ_BAND(x)  ((x) << 24)
 #define DSIM_PLL_EN(1 << 23)
-#define DSIM_PLL_P(x)  ((x) << 13)
+#define DSIM_PLL_P(x, offset)  ((x) << (offset))
 #define DSIM_PLL_M(x)  ((x) << 4)
 #define DSIM_PLL_S(x)  ((x) << 1)
 
@@ -368,6 +368,7 @@ static const struct samsung_dsim_driver_data 
exynos3_dsi_driver_data = {
.max_freq = 1000,
.wait_for_reset = 1,
.num_bits_resol = 11,
+   .pll_p_offset = 13,
.reg_values = reg_values,
 };
 
@@ -380,6 +381,7 @@ static const struct samsung_dsim_driver_data 
exynos4_dsi_driver_data = {
.max_freq = 1000,
.wait_for_reset = 1,
.num_bits_resol = 11,
+   .pll_p_offset = 13,
.reg_values = reg_values,
 };
 
@@ -390,6 +392,7 @@ static const struct samsung_dsim_driver_data 
exynos5_dsi_driver_data = {
.max_freq = 1000,
.wait_for_reset = 1,
.num_bits_resol = 11,
+   .pll_p_offset = 13,
.reg_values = reg_values,
 };
 
@@ -401,6 +404,7 @@ static const struct samsung_dsim_driver_data 
exynos5433_dsi_driver_data = {
.max_freq = 1500,
.wait_for_reset = 0,
.num_bits_resol = 12,
+   .pll_p_offset = 13,
.reg_values = exynos5433_reg_values,
 };
 
@@ -412,6 +416,7 @@ static const struct samsung_dsim_driver_data 
exynos5422_dsi_driver_data = {
.max_freq = 1500,
.wait_for_reset = 1,
.num_bits_resol = 12,
+   .pll_p_offset = 13,
.reg_values = exynos5422_reg_values,
 };
 
@@ -543,7 +548,8 @@ static unsigned long samsung_dsim_set_pll(struct 
samsung_dsim *dsi,
writel(driver_data->reg_values[PLL_TIMER],
dsi->reg_base + driver_data->plltmr_reg);
 
-   reg = DSIM_PLL_EN | DSIM_PLL_P(p) | DSIM_PLL_M(m) | DSIM_PLL_S(s);
+   reg = DSIM_PLL_EN | DSIM_PLL_P(p, driver_data->pll_p_offset) |
+ DSIM_PLL_M(m) | DSIM_PLL_S(s);
 
if (driver_data->has_freqband) {
static const unsigned long freq_bands[] = {
diff --git a/include/drm/bridge/samsung-dsim.h 
b/include/drm/bridge/samsung-dsim.h
index 0c5a905f3de7..df3d030daec6 100644
--- a/include/drm/bridge/samsung-dsim.h
+++ b/include/drm/bridge/samsung-dsim.h
@@ -53,6 +53,7 @@ struct samsung_dsim_driver_data {
unsigned int max_freq;
unsigned int wait_for_reset;
unsigned int num_bits_resol;
+   unsigned int pll_p_offset;
const unsigned int *reg_values;
 };
 
-- 
2.25.1

[PATCH v7 02/10] drm: bridge: samsung-dsim: Lookup OF-graph or Child node devices

The child devices in MIPI DSI can be binding with OF-graph
and also via child nodes.

The OF-graph interface represents the child devices via
remote and associated endpoint numbers like

dsi {
   compatible = "fsl,imx8mm-mipi-dsim";

   ports {
port@0 {
 reg = <0>;

 dsi_in_lcdif: endpoint@0 {
  reg = <0>;
  remote-endpoint = <&lcdif_out_dsi>;
 };
};

port@1 {
 reg = <1>;

 dsi_out_bridge: endpoint {
  remote-endpoint = <&bridge_in_dsi>;
 };
};
};

The child node interface represents the child devices via
conventional child nodes on given DSI parent like

dsi {
   compatible = "samsung,exynos5433-mipi-dsi";

   ports {
port@0 {
 reg = <0>;

 dsi_to_mic: endpoint {
  remote-endpoint = <&mic_to_dsi>;
 };
};
   };

   panel@0 {
reg = <0>;
   };
};

As Samsung DSIM bridge is common DSI IP across all Exynos DSI
and NXP i.MX8M host controllers, this patch adds support to
lookup the child devices whether its bindings on the associated
host represent OF-graph or child node interfaces.

v7, v6, v5, v4, v3:
* none

v2:
* new patch

Signed-off-by: Jagan Teki 
---
 drivers/gpu/drm/bridge/samsung-dsim.c | 38 +--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
b/drivers/gpu/drm/bridge/samsung-dsim.c
index 07563d00a420..c34c6abac815 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1356,18 +1356,52 @@ static int samsung_dsim_host_attach(struct 
mipi_dsi_host *host,
struct samsung_dsim *dsi = host_to_dsi(host);
const struct samsung_dsim_plat_data *pdata = dsi->plat_data;
struct device *dev = dsi->dev;
+   struct device_node *np = dev->of_node;
+   struct device_node *remote;
struct drm_panel *panel;
int ret;
 
-   panel = of_drm_find_panel(device->dev.of_node);
+   /**
+* Devices can also be child nodes when we also control that device
+* through the upstream device (ie, MIPI-DCS for a MIPI-DSI device).
+*
+* Lookup for a child node of the given parent that isn't either port
+* or ports.
+*/
+   for_each_available_child_of_node(np, remote) {
+   if (of_node_name_eq(remote, "port") ||
+   of_node_name_eq(remote, "ports"))
+   continue;
+
+   goto of_find_panel_or_bridge;
+   }
+
+   /*
+* of_graph_get_remote_node() produces a noisy error message if port
+* node isn't found and the absence of the port is a legit case here,
+* so at first we silently check whether graph presents in the
+* device-tree node.
+*/
+   if (!of_graph_is_present(np))
+   return -ENODEV;
+
+   remote = of_graph_get_remote_node(np, 1, 0);
+
+of_find_panel_or_bridge:
+   if (!remote)
+   return -ENODEV;
+
+   panel = of_drm_find_panel(remote);
if (!IS_ERR(panel)) {
dsi->out_bridge = devm_drm_panel_bridge_add(dev, panel);
} else {
-   dsi->out_bridge = of_drm_find_bridge(device->dev.of_node);
+   dsi->out_bridge = of_drm_find_bridge(remote);
if (!dsi->out_bridge)
dsi->out_bridge = ERR_PTR(-EINVAL);
}
 
+   of_node_put(remote);
+
if (IS_ERR(dsi->out_bridge)) {
ret = PTR_ERR(dsi->out_bridge);
DRM_DEV_ERROR(dev, "failed to find the bridge: %d\n", ret);
-- 
2.25.1

[PATCH v7 07/10] drm: bridge: samsung-dsim: Add atomic_get_input_bus_fmts

Finding the right input bus format throughout the pipeline is hard
so add atomic_get_input_bus_fmts callback and initialize with the
default RGB888_1X24 bus format on DSI-end.

This format can be used in pipeline for negotiating bus format between
the DSI-end of this bridge and the other component closer to pipeline
components.

v7, v6, v5, v4:
* none

v3:
* include media-bus-format.h

v2:
* none

v1:
* new patch

Signed-off-by: Jagan Teki 
---
 drivers/gpu/drm/bridge/samsung-dsim.c | 28 +++
 1 file changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
b/drivers/gpu/drm/bridge/samsung-dsim.c
index 41970e794a7c..f714e49c1eab 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -1321,6 +1322,32 @@ static void samsung_dsim_atomic_post_disable(struct 
drm_bridge *bridge,
pm_runtime_put_sync(dsi->dev);
 }
 
+#define MAX_INPUT_SEL_FORMATS  1
+
+static u32 *
+samsung_dsim_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
+  struct drm_bridge_state *bridge_state,
+  struct drm_crtc_state *crtc_state,
+  struct drm_connector_state *conn_state,
+  u32 output_fmt,
+  unsigned int *num_input_fmts)
+{
+   u32 *input_fmts;
+
+   *num_input_fmts = 0;
+
+   input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
+GFP_KERNEL);
+   if (!input_fmts)
+   return NULL;
+
+   /* This is the DSI-end bus format */
+   input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
+   *num_input_fmts = 1;
+
+   return input_fmts;
+}
+
 static int samsung_dsim_atomic_check(struct drm_bridge *bridge,
 struct drm_bridge_state *bridge_state,
 struct drm_crtc_state *crtc_state,
@@ -1384,6 +1411,7 @@ static const struct drm_bridge_funcs 
samsung_dsim_bridge_funcs = {
.atomic_duplicate_state = 
drm_atomic_helper_bridge_duplicate_state,
.atomic_destroy_state   = 
drm_atomic_helper_bridge_destroy_state,
.atomic_reset   = drm_atomic_helper_bridge_reset,
+   .atomic_get_input_bus_fmts  = 
samsung_dsim_atomic_get_input_bus_fmts,
.atomic_check   = samsung_dsim_atomic_check,
.atomic_pre_enable  = samsung_dsim_atomic_pre_enable,
.atomic_enable  = samsung_dsim_atomic_enable,
-- 
2.25.1

[PATCH v7 04/10] drm: bridge: samsung-dsim: Handle proper DSI host initialization

DSI host initialization handling in previous exynos dsi driver has
some pitfalls. It initializes the host during host transfer() hook
that is indeed not the desired call flow for I2C and any other DSI
configured downstream bridges.

Host transfer() is usually triggered for downstream DSI panels or
bridges and I2C-configured-DSI bridges miss these host initialization
as these downstream bridges use bridge operations hooks like pre_enable,
and enable in order to initialize or set up the host.

This patch is trying to handle the host init handler to satisfy all
downstream panels and bridges. Added the DSIM_STATE_REINITIALIZED state
flag to ensure that host init is also done on first cmd transfer, this
helps existing DSI panels work on exynos platform (form Marek
Szyprowski).

v7, v6, v5:
* none

v4:
* update init handling to ensure host init done on first cmd transfer

v3:
* none

v2:
* check initialized state in samsung_dsim_init

v1:
* keep DSI init in host transfer

Signed-off-by: Marek Szyprowski 
Signed-off-by: Jagan Teki 
---
 drivers/gpu/drm/bridge/samsung-dsim.c | 25 +
 include/drm/bridge/samsung-dsim.h |  5 +++--
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
b/drivers/gpu/drm/bridge/samsung-dsim.c
index 1bae3673151b..563781bb27c4 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1234,12 +1234,17 @@ static void samsung_dsim_disable_irq(struct 
samsung_dsim *dsi)
disable_irq(dsi->irq);
 }
 
-static int samsung_dsim_init(struct samsung_dsim *dsi)
+static int samsung_dsim_init(struct samsung_dsim *dsi, unsigned int flag)
 {
const struct samsung_dsim_driver_data *driver_data = dsi->driver_data;
 
+   if (dsi->state & flag)
+   return 0;
+
samsung_dsim_reset(dsi);
-   samsung_dsim_enable_irq(dsi);
+
+   if (!(dsi->state & DSIM_STATE_INITIALIZED))
+   samsung_dsim_enable_irq(dsi);
 
if (driver_data->reg_values[RESET_TYPE] == DSIM_FUNCRST)
samsung_dsim_enable_lane(dsi, BIT(dsi->lanes) - 1);
@@ -1250,6 +1255,8 @@ static int samsung_dsim_init(struct samsung_dsim *dsi)
samsung_dsim_set_phy_ctrl(dsi);
samsung_dsim_init_link(dsi);
 
+   dsi->state |= flag;
+
return 0;
 }
 
@@ -1269,6 +1276,10 @@ static void samsung_dsim_atomic_pre_enable(struct 
drm_bridge *bridge,
}
 
dsi->state |= DSIM_STATE_ENABLED;
+
+   ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED);
+   if (ret)
+   return;
 }
 
 static void samsung_dsim_atomic_enable(struct drm_bridge *bridge,
@@ -1458,12 +1469,9 @@ static ssize_t samsung_dsim_host_transfer(struct 
mipi_dsi_host *host,
if (!(dsi->state & DSIM_STATE_ENABLED))
return -EINVAL;
 
-   if (!(dsi->state & DSIM_STATE_INITIALIZED)) {
-   ret = samsung_dsim_init(dsi);
-   if (ret)
-   return ret;
-   dsi->state |= DSIM_STATE_INITIALIZED;
-   }
+   ret = samsung_dsim_init(dsi, DSIM_STATE_REINITIALIZED);
+   if (ret)
+   return ret;
 
ret = mipi_dsi_create_packet(&xfer.packet, msg);
if (ret < 0)
@@ -1653,6 +1661,7 @@ static int __maybe_unused samsung_dsim_suspend(struct 
device *dev)
 
if (dsi->state & DSIM_STATE_INITIALIZED) {
dsi->state &= ~DSIM_STATE_INITIALIZED;
+   dsi->state &= ~DSIM_STATE_REINITIALIZED;
 
samsung_dsim_disable_clock(dsi);
 
diff --git a/include/drm/bridge/samsung-dsim.h 
b/include/drm/bridge/samsung-dsim.h
index b8132bf8e36f..0c5a905f3de7 100644
--- a/include/drm/bridge/samsung-dsim.h
+++ b/include/drm/bridge/samsung-dsim.h
@@ -17,8 +17,9 @@ struct samsung_dsim;
 
 #define DSIM_STATE_ENABLED BIT(0)
 #define DSIM_STATE_INITIALIZED BIT(1)
-#define DSIM_STATE_CMD_LPM BIT(2)
-#define DSIM_STATE_VIDOUT_AVAILABLEBIT(3)
+#define DSIM_STATE_REINITIALIZED   BIT(2)
+#define DSIM_STATE_CMD_LPM BIT(3)
+#define DSIM_STATE_VIDOUT_AVAILABLEBIT(4)
 
 enum samsung_dsim_type {
SAMSUNG_DSIM_TYPE_EXYNOS3250,
-- 
2.25.1

[PATCH v7 03/10] drm: bridge: samsung-dsim: Mark PHY as optional

In i.MX8M Mini/Nano SoC the DSI Phy requires a MIPI DPHY bit
to reset in order to activate the PHY and that can be done via
upstream i.MX8M blk-ctrl driver.

So, mark the phy get as optional.

v7, v6, v5, v4, v3, v2:
* none

v1:
* new patch

Signed-off-by: Jagan Teki 
---
 drivers/gpu/drm/bridge/samsung-dsim.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
b/drivers/gpu/drm/bridge/samsung-dsim.c
index c34c6abac815..1bae3673151b 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1584,7 +1584,7 @@ int samsung_dsim_probe(struct platform_device *pdev)
if (IS_ERR(dsi->reg_base))
return PTR_ERR(dsi->reg_base);
 
-   dsi->phy = devm_phy_get(dev, "dsim");
+   dsi->phy = devm_phy_optional_get(dev, "dsim");
if (IS_ERR(dsi->phy)) {
dev_info(dev, "failed to get dsim phy\n");
return PTR_ERR(dsi->phy);
-- 
2.25.1

[PATCH v7 01/10] drm: bridge: Add Samsung DSIM bridge driver

Samsung MIPI DSIM controller is common DSI IP that can be used in various
SoCs like Exynos, i.MX8M Mini/Nano.

In order to access this DSI controller between various platform SoCs,
the ideal way to incorporate this in the drm stack is via the drm bridge
driver.

This patch is trying to differentiate platform-specific and bridge driver
code by maintaining exynos platform glue code in exynos_drm_dsi.c driver
and common bridge driver code in samsung-dsim.c providing that the new
platform-specific glue should be supported in the bridge driver, unlike
exynos platform drm drivers.

- Add samsung_dsim_plat_data for keeping platform-specific attributes like
  host_ops, irq_ops, and hw_type.

- Initialize the plat_data hooks for exynos platform in exynos_drm_dsi.c.

- samsung_dsim_probe is the common probe call across exynos_drm_dsi.c and
  samsung-dsim.c.

- plat_data hooks like host_ops and irq_ops are invoked during the
  respective bridge call chains.

v7:
* fix the drm bridge attach chain for exynos drm dsi driver
* fix the hw_type checking logic

v6:
* handle previous bridge pointer for exynos dsi

v5:
* [mszyprow] reworked driver initialization using the same approach as in
  drivers/gpu/drm/{exynos/exynos_dp.c, bridge/analogix/analogix_dp_core.c},
  removed weak functions, moved exynos_dsi_driver back to exynos_drm_dsi.c
  and restored integration with exynos_drm custom initialization.
* updated the commit message [Jagan]

v4:
* include Inki Dae in MAINTAINERS
* remove dsi_driver probe in exynos_drm_drv to support multi-arch build

v3:
* restore gpio related fixes
* restore proper bridge chain
* rework initialization issue
* fix header includes in proper way

v2:
* fixed exynos dsi driver conversion (Marek Szyprowski)
* updated commit message
* updated MAINTAINERS file

v1:
* don't maintain component_ops in bridge driver
* don't maintain platform glue code in bridge driver
* add platform-specific glue code and make a common bridge

Signed-off-by: Marek Szyprowski 
Signed-off-by: Jagan Teki 
---
 MAINTAINERS |9 +
 drivers/gpu/drm/bridge/Kconfig  |   12 +
 drivers/gpu/drm/bridge/Makefile |1 +
 drivers/gpu/drm/bridge/samsung-dsim.c   | 1703 ++
 drivers/gpu/drm/exynos/Kconfig  |1 +
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 1766 ++-
 include/drm/bridge/samsung-dsim.h   |  113 ++
 7 files changed, 1952 insertions(+), 1653 deletions(-)
 create mode 100644 drivers/gpu/drm/bridge/samsung-dsim.c
 create mode 100644 include/drm/bridge/samsung-dsim.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 9ae989b32ebb..ba7a6721443c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6624,6 +6624,15 @@ T:   git git://anongit.freedesktop.org/drm/drm-misc
 F: Documentation/devicetree/bindings/display/panel/samsung,lms397kf04.yaml
 F: drivers/gpu/drm/panel/panel-samsung-db7430.c
 
+DRM DRIVER FOR SAMSUNG MIPI DSIM BRIDGE
+M: Jagan Teki 
+M: Marek Szyprowski 
+M: Inki Dae 
 S: Maintained
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 57946d80b02d..8e85dac9f53e 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -231,6 +231,18 @@ config DRM_PARADE_PS8640
  The PS8640 is a high-performance and low-power
  MIPI DSI to eDP converter
 
+config DRM_SAMSUNG_DSIM
+   tristate "Samsung MIPI DSIM bridge driver"
+   depends on COMMON_CLK
+   depends on OF && HAS_IOMEM
+   select DRM_KMS_HELPER
+   select DRM_MIPI_DSI
+   select DRM_PANEL_BRIDGE
+   help
+ The Samsung MIPI DSIM bridge controller driver.
+ This MIPI DSIM bridge can be found it on Exynos SoCs and
+ NXP's i.MX8M Mini/Nano.
+
 config DRM_SIL_SII8620
tristate "Silicon Image SII8620 HDMI/MHL bridge"
depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 1884803c6860..dae843723991 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_DRM_MEGACHIPS_STDP_GE_B850V3_FW) += 
megachips-stdp-ge-b850v
 obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
 obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
 obj-$(CONFIG_DRM_PARADE_PS8640) += parade-ps8640.o
+obj-$(CONFIG_DRM_SAMSUNG_DSIM) += samsung-dsim.o
 obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o
 obj-$(CONFIG_DRM_SII902X) += sii902x.o
 obj-$(CONFIG_DRM_SII9234) += sii9234.o
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
b/drivers/gpu/drm/bridge/samsung-dsim.c
new file mode 100644
index ..07563d00a420
--- /dev/null
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -0,0 +1,1703 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Samsung MIPI DSIM bridge driver.
+ *
+ * Copyright (C) 2021 Amarula Solutions(India)
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd
+ * Author: Jagan Teki 
+ *
+ * Based on exynos_drm_dsi from
+ * Tomasz Figa 
+

[PATCH v7 00/10] drm: bridge: Add Samsung MIPI DSIM bridge

This series supports common bridge support for Samsung MIPI DSIM
which is used in Exynos and i.MX8MM SoC's.

The final bridge supports both the Exynos and i.MX8MM DSI devices.

Changes for v7:
* fix the drm bridge attach chain for exynos drm dsi driver
* fix the hw_type checking logic

Changes for v6:
* handle previous bridge for exynos dsi while attaching bridge 

Changes for v5:
* bridge changes to support multi-arch
* updated and clear commit messages
* add hw_type via plat data
* removed unneeded quirk
* rebased on linux-next

Changes for v4:
* include Inki Dae in MAINTAINERS
* remove dsi_driver probe in exynos_drm_drv to support multi-arch build
* update init handling to ensure host init done on first cmd transfer

Changes for v3:
* fix the mult-arch build
* fix dsi host init
* updated commit messages

Changes for v2:
* fix bridge handling
* fix dsi host init
* correct the commit messages

Patch 0001: Samsung DSIM bridge

Patch 0002: PHY optional

Patch 0003: OF-graph or Child node lookup

Patch 0004: DSI host initialization 

Patch 0005: atomic check

Patch 0006: PMS_P offset via plat data

Patch 0007: atomic_get_input_bus_fmts

Patch 0008: input_bus_flags

Patch 0009: document fsl,imx8mm-mipi-dsim

Patch 0010: add i.MX8MM DSIM support

Tested in Engicam i.Core MX8M Mini SoM.

Repo:
https://gitlab.com/openedev/kernel/-/commits/imx8mm-dsi-v7

Any inputs?
Jagan.

Jagan Teki (10):
  drm: bridge: Add Samsung DSIM bridge driver
  drm: bridge: samsung-dsim: Lookup OF-graph or Child node devices
  drm: bridge: samsung-dsim: Mark PHY as optional
  drm: bridge: samsung-dsim: Handle proper DSI host initialization
  drm: bridge: samsung-dsim: Add atomic_check
  drm: bridge: samsung-dsim: Add platform PLL_P (PMS_P) offset
  drm: bridge: samsung-dsim: Add atomic_get_input_bus_fmts
  drm: bridge: samsung-dsim: Add input_bus_flags
  dt-bindings: display: exynos: dsim: Add NXP i.MX8MM support
  drm: bridge: samsung-dsim: Add i.MX8MM support

 .../bindings/display/exynos/exynos_dsim.txt   |1 +
 MAINTAINERS   |9 +
 drivers/gpu/drm/bridge/Kconfig|   12 +
 drivers/gpu/drm/bridge/Makefile   |1 +
 drivers/gpu/drm/bridge/samsung-dsim.c | 1856 +
 drivers/gpu/drm/exynos/Kconfig|1 +
 drivers/gpu/drm/exynos/exynos_drm_dsi.c   | 1766 +---
 include/drm/bridge/samsung-dsim.h |  115 +
 8 files changed, 2108 insertions(+), 1653 deletions(-)
 create mode 100644 drivers/gpu/drm/bridge/samsung-dsim.c
 create mode 100644 include/drm/bridge/samsung-dsim.h

-- 
2.25.1

Re: [PATCH 15/16] drm/udl: Add register constants for framebuffer scanout addresses


Hi

Am 05.10.22 um 00:59 schrieb Javier Martinez Canillas:

On 9/19/22 15:04, Thomas Zimmermann wrote:

Add register constants for the framebuffer scanout addresses and
update the related helper functions. No functional changes.

Signed-off-by: Thomas Zimmermann 
---


Reviewed-by: Javier Martinez Canillas 

[...]


+   u8 reg20 = (base & 0xff) >> 16;
+   u8 reg21 = (base & 0x00ff00) >> 8;
+   u8 reg22 = (base & 0xff);
+


Maybe would be cleaner to use the FIELD_PREP() and GENMASK() macros instead ?



Thank you for reviewing my patchset. I'll update the patch with these 
macros.


Best regards
Thomas


--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH v1 17/17] drm/mediatek: Add mt8195-dpi support to drm_drv

2022-10-05 Thread Krzysztof Kozlowski

On 05/10/2022 11:34, Guillaume Ranquet wrote:
> On Tue, 04 Oct 2022 17:05, Krzysztof Kozlowski
>  wrote:
>> On 04/10/2022 13:55, Guillaume Ranquet wrote:
 No. You said what the code is doing. I think I understand this. You
 still do not need more compatibles. Your sentence did not clarify it
 because it did not answer at all to question "why". Why do you need it?

 Sorry, the change looks not correct.

 Best regards,
 Krzysztof

>>>
>>> I need a new compatible to adress the specifics of mt8195 in the mtk_dpi 
>>> driver,
>>> the change is in this series with:
>>> [PATCH v1 16/17] drm/mediatek: dpi: Add mt8195 hdmi to DPI driver [1]
>>
>> But you do not have specifics of mt8195. I wrote it in the beginning.
>>
>>>
>>> I then need to add that compatible to the "list" here in mtk_drm_drv.
>>
>> No, you do not... I checked the driver and there is no single need... or
>> convince me you need.
>>
>>> I don't see a way around this unless I rewrite the way mtk_drm_drv works?
>>
>> Why rewrite? You have all compatibles in place.
>>
>>>
>>> Maybe if I declare a new compatible that is generic to all mediatek
>>> dpi variants?
>>
>> You were asked to use fallback. Don't create some fake fallbacks. Use
>> existing ones.
>>
>>> and have all the dts specify the node with both the generic dpi and
>>> the specific compatible?
>>>
>>> dpi@xxx {
>>> compatible = "mediatek,dpi", "mediatek,mt8195-dpi";
>>
>> I don't know what's this but certainly looks odd. Some wild-card
>> compatible in front (not fallback) and none are documented.
>>
>>> ...
>>> }
>>>
>>> Then I can "collapse" all the dpi related nodes in mtk_drm_drv under
>>> "mediatek,dpi" ?
>>>
>>> I guess would have to do the change for all other components that are 
>>> needed in
>>> mtk_drm_drv (mmsys, aal, ccor, color, dither, dsc, gamma, mutex...).
>>>
>>> That's the only trivial way I can think of implementing this with the
>>> current status
>>> of the mtk_drm stack.
>>>
>>> Do you have any other ideas in mind?
>>
>> Use fallback of compatible device. That's the common pattern.
>> Everywhere, Mediatek as well.
>>
> 
> I'm unsure about what a "fallback of compatible device" is.
> But from what I can gather, you'd have me write the dts as:
> 
> dpi@xxx {
>   compatible = "mediatek,mt8195-dpi", "mediatek,mt2701-dpi";
>   ...
> }

Sounds reasonable. Just someone should check whether devices are
actually compatible, because driver is just a hint.

> 
> so that the mtk_dpi driver will use the specific mt8195 dpi config and
> the mtk_drm driver will fallback to mt2701 to find the compatible it needs.
> 
> Would you like me to use this pattern for all the other compatibles declared
> in the mtk_ddp_comp_ids array in separate patches?

Could be for consistency, but it is up to you.

Best regards,
Krzysztof

Re: [PATCH 4/5] drm/msm/dpu1: Account for DSC's bits_per_pixel having 4 fractional bits





On 10/4/2022 3:11 PM, Marijn Suijten wrote:

On 2022-10-04 10:03:07, Abhinav Kumar wrote:



On 10/1/2022 12:08 PM, Marijn Suijten wrote:

According to the comment this DPU register contains the bits per pixel
as a 6.4 fractional value, conveniently matching the contents of
bits_per_pixel in struct drm_dsc_config which also uses 4 fractional
bits.  However, the downstream source this implementation was
copy-pasted from has its bpp field stored _without_ fractional part.

This makes the entire convoluted math obsolete as it is impossible to
pull those 4 fractional bits out of thin air, by somehow trying to reuse
the lowest 2 bits of a non-fractional bpp (lsb = bpp % 4??).

The rest of the code merely attempts to keep the integer part a multiple
of 4, which is rendered useless thanks to data |= dsc->bits_per_pixel <<
12; already filling up those bits anyway (but not on downstream).

Fixes: c110cfd1753e ("drm/msm/disp/dpu1: Add support for DSC")
Signed-off-by: Marijn Suijten 


Many of this bugs are because the downstream code from which this
implementation was derived wasnt the latest perhaps?


Perhaps, this code is "identical" to what I'm looking at in some
downstream 4.14 / 4.19, where the upstream struct for DSC either wasn't
there or wasn't used.  We have to find and address these bugs one by one
to make our panels work, and this series gets one platform (sdm845) down
but has more work pending for others (sm8250 has my current focus).

Or are you suggesting to "redo" the DSC integration work based on a
(much) newer display techpack (SDE driver)?


There is no need to redo the DSC integration now.

The code I am referring to is here :

https://git.codelinaro.org/clo/la/platform/vendor/opensource/display-drivers/-/blob/DISPLAY.LA.2.0.r1-08000-WAIPIO.0/msm/sde_dsc_helper.c#L240

So with respect to the redundant math in patches 1/3/4/5 of this series, 
I dont see all the redundant math anymore in this calculation.


This is what i meant by my comment.

When DSC changes were pushed, they were indeed validated on sdm845 
devices by Vinod so there was a certain level of confidence on those 
changes.


At this point, we should just consider these as bug-fixes for upstream 
and keep going. A full redo is not required.


At some point in the next couple of months, we plan to add DSC 1.2 
support to MSM.


We will check for any missing changes (if any after this series of 
yours) and push those as part of that.





Earlier, downstream had its own DSC struct maybe leading to this
redundant math but now we have migrated over to use the upstream struct
drm_dsc_config.


Found the 3-year-old `disp: msm: use upstream dsc config data` commit
that makes this change.  It carries a similar comment:

 /* integer bpp support only */

The superfluous math was howerver removed earlier, in:

 disp: msm: fix dsc parameters related to 10 bpc 10 bpp

- Marijn


That being said, this patch LGTM
Reviewed-by: Abhinav Kumar

Re: [PATCH RESEND] drm/i915: Fix display problems after resume

2022-10-05 Thread Matthew Auld


Hi Thomas,

On 05/10/2022 13:11, Thomas Hellström wrote:

Commit 39a2bd34c933 ("drm/i915: Use the vma resource as argument for gtt
binding / unbinding") introduced a regression that due to the vma resource
tracking of the binding state, dpt ptes were not correctly repopulated.
Fix this by clearing the vma resource state before repopulating.
The state will subsequently be restored by the bind_vma operation.

Fixes: 39a2bd34c933 ("drm/i915: Use the vma resource as argument for gtt binding / 
unbinding")
Signed-off-by: Thomas Hellström 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20220912121957.31310-1-thomas.hellst...@linux.intel.com
Cc: Matthew Auld 
Cc: intel-...@lists.freedesktop.org
Cc:  # v5.18+
Reported-and-tested-by: Kevin Boulain 
Tested-by: David de Sousa 

Reviewed-by: Matthew Auld

Re: [PATCH v2 0/2] drm/rockchip: dw_hdmi: Add 4k@30 support

2022-10-05 Thread Robin Murphy


On 2022-10-05 12:10, Sascha Hauer wrote:

On Wed, Oct 05, 2022 at 12:51:57PM +0200, Dan Johansen wrote:


Den 05.10.2022 kl. 12.06 skrev Sascha Hauer:

On Wed, Sep 28, 2022 at 10:39:27AM +0200, Dan Johansen wrote:

Den 28.09.2022 kl. 10.37 skrev Sascha Hauer:

On Tue, Sep 27, 2022 at 07:53:54PM +0200, Dan Johansen wrote:

Den 26.09.2022 kl. 12.30 skrev Michael Riesch:

Hi Sascha,

On 9/26/22 10:04, Sascha Hauer wrote:

This series adds support for 4k@30 to the rockchip HDMI controller. This
has been tested on a rk3568 rock3a board. It should be possible to add
4k@60 support the same way, but it doesn't work for me, so let's add
4k@30 as a first step.

 Sascha

Changes since v1:
- Allow non standard clock rates only on Synopsys phy as suggested by
  Robin Murphy

Sascha Hauer (2):
  drm/rockchip: dw_hdmi: relax mode_valid hook
  drm/rockchip: dw_hdmi: Add support for 4k@30 resolution

 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 34 -
 1 file changed, 27 insertions(+), 7 deletions(-)

Thanks for the v2! On a RK3568 EVB1 with a HP 27f 4k monitor

Tested-by: Michael Riesch 

Sadly this still doesn't give my display out on my 2k monitor. Not even just
1080p picture like the old current implementation does.

By "like the old current implementation" you mean that this patchset
introduces a regression for you?

Yes. What currently in the kernel at least shows as 1080p on my 2K monitor,
while this patchset turns off the screen.

Which SoC are you testing this on? I assume RK3568, right? Which patch
introduces that regression, the first or the second one?

I tested on the Odroid M, which is rk3568.
I have only applied them both, as I was under the impression that both are
needed for the 4k support.


Yes, both I needed, but I am interested which one introduces the
regression as I can't reproduce it.


One thing that might be worthwhile is to compare what "drm.debug=4" 
output says about the chosen mode and its clock rate vs. what 
/sys/kernel/debug/clk/clk_summary says about how things ended up in 
practice, to see whether it's a case of the clock not being able to get 
close enough to the correct rate at all.


Robin.

[PATCH RESEND] drm/i915: Fix display problems after resume

2022-10-05 Thread Thomas Hellström

Commit 39a2bd34c933 ("drm/i915: Use the vma resource as argument for gtt
binding / unbinding") introduced a regression that due to the vma resource
tracking of the binding state, dpt ptes were not correctly repopulated.
Fix this by clearing the vma resource state before repopulating.
The state will subsequently be restored by the bind_vma operation.

Fixes: 39a2bd34c933 ("drm/i915: Use the vma resource as argument for gtt 
binding / unbinding")
Signed-off-by: Thomas Hellström 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20220912121957.31310-1-thomas.hellst...@linux.intel.com
Cc: Matthew Auld 
Cc: intel-...@lists.freedesktop.org
Cc:  # v5.18+
Reported-and-tested-by: Kevin Boulain 
Tested-by: David de Sousa 
---
 drivers/gpu/drm/i915/gt/intel_ggtt.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c 
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index b31fe0fb013f..5c67e49aacf6 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -1275,10 +1275,16 @@ bool i915_ggtt_resume_vm(struct i915_address_space *vm)
atomic_read(&vma->flags) & I915_VMA_BIND_MASK;
 
GEM_BUG_ON(!was_bound);
-   if (!retained_ptes)
+   if (!retained_ptes) {
+   /*
+* Clear the bound flags of the vma resource to allow
+* ptes to be repopulated.
+*/
+   vma->resource->bound_flags = 0;
vma->ops->bind_vma(vm, NULL, vma->resource,
   obj ? obj->cache_level : 0,
   was_bound);
+   }
if (obj) { /* only used during resume => exclusive access */
write_domain_objs |= fetch_and_zero(&obj->write_domain);
obj->read_domains |= I915_GEM_DOMAIN_GTT;
-- 
2.37.3

[PATCH v4 1/2] drm/atomic-helper: Don't allocated plane state in CRTC check

In drm_atomic_helper_check_crtc_state(), do not add a new plane state
to the global state if it does not exist already. Adding a new plane
state will result in overhead for the plane during the atomic-commit
step.

For the test in drm_atomic_helper_check_crtc_state() to succeed, it
is important that the CRTC has an enabled primary plane after the
commit. Simply testing the CRTC state's plane_mask for a primary plane
is sufficient.

Note that the helper still only tests for an attached primary plane.
Drivers have to ensure that the plane contains valid pixel information.

v3:
* test for a primary plane in plane_mask (Ville)
v2:
* remove unnecessary test for plane->crtc (Ville)
* inline drm_atomic_get_next_plane_state() (Ville)
* acquire plane lock before accessing plane->state (Ville)

Signed-off-by: Thomas Zimmermann 
Fixes: d6b9af1097fe ("drm/atomic-helper: Add helper 
drm_atomic_helper_check_crtc_state()")
Cc: Thomas Zimmermann 
Cc: Jocelyn Falempe 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/drm_atomic_helper.c | 10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 98cc3137c062..02b4a7dc92f5 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -945,7 +945,6 @@ int drm_atomic_helper_check_crtc_state(struct 
drm_crtc_state *crtc_state,
   bool can_disable_primary_planes)
 {
struct drm_device *dev = crtc_state->crtc->dev;
-   struct drm_atomic_state *state = crtc_state->state;
 
if (!crtc_state->enable)
return 0;
@@ -956,14 +955,7 @@ int drm_atomic_helper_check_crtc_state(struct 
drm_crtc_state *crtc_state,
struct drm_plane *plane;
 
drm_for_each_plane_mask(plane, dev, crtc_state->plane_mask) {
-   struct drm_plane_state *plane_state;
-
-   if (plane->type != DRM_PLANE_TYPE_PRIMARY)
-   continue;
-   plane_state = drm_atomic_get_plane_state(state, plane);
-   if (IS_ERR(plane_state))
-   return PTR_ERR(plane_state);
-   if (plane_state->fb && plane_state->crtc) {
+   if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
has_primary_plane = true;
break;
}
-- 
2.37.3

[PATCH v4 2/2] drm/atomic-helper: Replace drm_atomic_helper_check_crtc_state()

Rename the atomic helper function drm_atomic_helper_check_crtc_state()
to drm_atomic_helper_check_crtc_primary_plane() and only check for an
attached primary plane. Adapt callers.

Instead of having one big function to check for various CRTC state
conditions, we rather want smaller functions that drivers can pick
individually.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/ast/ast_mode.c  |  8 ++--
 drivers/gpu/drm/drm_atomic_helper.c | 52 +
 drivers/gpu/drm/drm_simple_kms_helper.c |  6 ++-
 drivers/gpu/drm/mgag200/mgag200_mode.c  |  8 ++--
 drivers/gpu/drm/solomon/ssd130x.c   |  6 ++-
 drivers/gpu/drm/tiny/simpledrm.c|  6 ++-
 include/drm/drm_atomic_helper.h |  3 +-
 7 files changed, 42 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 1bc0220e6783..d598b1e1447f 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -1129,13 +1129,13 @@ static int ast_crtc_helper_atomic_check(struct drm_crtc 
*crtc,
bool succ;
int ret;
 
-   ret = drm_atomic_helper_check_crtc_state(crtc_state, false);
-   if (ret)
-   return ret;
-
if (!crtc_state->enable)
goto out;
 
+   ret = drm_atomic_helper_check_crtc_primary_plane(crtc_state);
+   if (ret)
+   return ret;
+
ast_state = to_ast_crtc_state(crtc_state);
 
format = ast_state->format;
diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 02b4a7dc92f5..1a586b3c454b 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -924,51 +924,35 @@ int drm_atomic_helper_check_plane_state(struct 
drm_plane_state *plane_state,
 EXPORT_SYMBOL(drm_atomic_helper_check_plane_state);
 
 /**
- * drm_atomic_helper_check_crtc_state() - Check CRTC state for validity
+ * drm_atomic_helper_check_crtc_primary_plane() - Check CRTC state for primary 
plane
  * @crtc_state: CRTC state to check
- * @can_disable_primary_planes: can the CRTC be enabled without a primary 
plane?
  *
- * Checks that a desired CRTC update is valid. Drivers that provide
- * their own CRTC handling rather than helper-provided implementations may
- * still wish to call this function to avoid duplication of error checking
- * code.
- *
- * Note that @can_disable_primary_planes only tests if the CRTC can be
- * enabled without a primary plane. To test if a primary plane can be updated
- * without a CRTC, use drm_atomic_helper_check_plane_state() in the plane's
- * atomic check.
+ * Checks that a CRTC has at least one primary plane attached to it, which is
+ * a requirement on some hardware. Note that this only involves the CRTC side
+ * of the test. To test if the primary plane is visible or if it can be updated
+ * without the CRTC being enabled, use drm_atomic_helper_check_plane_state() in
+ * the plane's atomic check.
  *
  * RETURNS:
- * Zero if update appears valid, error code on failure
+ * 0 if a primary plane is attached to the CRTC, or an error code otherwise
  */
-int drm_atomic_helper_check_crtc_state(struct drm_crtc_state *crtc_state,
-  bool can_disable_primary_planes)
+int drm_atomic_helper_check_crtc_primary_plane(struct drm_crtc_state 
*crtc_state)
 {
-   struct drm_device *dev = crtc_state->crtc->dev;
-
-   if (!crtc_state->enable)
-   return 0;
+   struct drm_crtc *crtc = crtc_state->crtc;
+   struct drm_device *dev = crtc->dev;
+   struct drm_plane *plane;
 
/* needs at least one primary plane to be enabled */
-   if (!can_disable_primary_planes) {
-   bool has_primary_plane = false;
-   struct drm_plane *plane;
-
-   drm_for_each_plane_mask(plane, dev, crtc_state->plane_mask) {
-   if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
-   has_primary_plane = true;
-   break;
-   }
-   }
-   if (!has_primary_plane) {
-   drm_dbg_kms(dev, "Cannot enable CRTC without a primary 
plane.\n");
-   return -EINVAL;
-   }
+   drm_for_each_plane_mask(plane, dev, crtc_state->plane_mask) {
+   if (plane->type == DRM_PLANE_TYPE_PRIMARY)
+   return 0;
}
 
-   return 0;
+   drm_dbg_atomic(dev, "[CRTC:%d:%s] primary plane missing\n", 
crtc->base.id, crtc->name);
+
+   return -EINVAL;
 }
-EXPORT_SYMBOL(drm_atomic_helper_check_crtc_state);
+EXPORT_SYMBOL(drm_atomic_helper_check_crtc_primary_plane);
 
 /**
  * drm_atomic_helper_check_planes - validate state object for planes changes
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c 
b/drivers/gpu/drm/drm_simple_kms_helper.c
index e9f782119d3d..31233c6ae3c4 100644
--- a/drivers/gpu/drm/drm_simple_kms_helper.c
+++ b/driv

[PATCH v4 0/2] drm/atomic-helpers: Fix CRTC primary-plane test

(was: drm/atomic-helper: Don't allocated plane state in CRTC check)

Fix the test for a CRTC's primary plane and clean up the test function
to only do the test. Up to v3, this patchset was a single patch, but
the cleanup turns it into a series.

v4:
* clean up the helper function (Ville)

Thomas Zimmermann (2):
  drm/atomic-helper: Don't allocated plane state in CRTC check
  drm/atomic-helper: Replace drm_atomic_helper_check_crtc_state()

 drivers/gpu/drm/ast/ast_mode.c  |  8 ++--
 drivers/gpu/drm/drm_atomic_helper.c | 60 -
 drivers/gpu/drm/drm_simple_kms_helper.c |  6 ++-
 drivers/gpu/drm/mgag200/mgag200_mode.c  |  8 ++--
 drivers/gpu/drm/solomon/ssd130x.c   |  6 ++-
 drivers/gpu/drm/tiny/simpledrm.c|  6 ++-
 include/drm/drm_atomic_helper.h |  3 +-
 7 files changed, 42 insertions(+), 55 deletions(-)

-- 
2.37.3

Re: [PATCH 1/2] drm/i915/display: fix randconfig build

On Wed, 05 Oct 2022, Jiri Slaby  wrote:
> On 04. 10. 22, 12:52, Jani Nikula wrote:
>> On Tue, 04 Oct 2022, "Jiri Slaby (SUSE)"  wrote:
>>> When DRM_I915=y and BACKLIGHT_CLASS_DEVICE=m, the build fails:
>>> ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function 
>>> `intel_backlight_device_register':
>>> intel_backlight.c:(.text+0x5587): undefined reference to 
>>> `backlight_device_get_by_name'
>>>
>>> ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function 
>>> `intel_backlight_device_unregister':
>>> intel_backlight.c:(.text+0x576e): undefined reference to 
>>> `backlight_device_unregister'
>>>
>>> To fix this, use IS_REACHABLE(), not IS_ENABLED() in backlight. That is,
>>> with the above config, backlight support is disabled.
>> 
>> So I don't want this. I'll take a patch that fixes the dependencies to
>> block DRM_I915=y and BACKLIGHT_CLASS_DEVICE=m. Nobody wants that combo,
>> and IMO using IS_REACHABLE() is a workaround to hide a broken config
>> under the carpet.
>> 
>> The right thing to do is
>> 
>> config DRM_I915
>>  depends on BACKLIGHT_CLASS_DEVICE || BACKLIGHT_CLASS_DEVICE=n.
>> 
>> We're selecting BACKLIGHT_CLASS_DEVICE because almost everyone else is
>> too, and a combo of selecting and depending leads to circular
>> dependencies. But depending is the right fix.
>
> I'm not sure what should I do now. If I do:
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -4,6 +4,7 @@ config DRM_I915
>  depends on DRM
>  depends on X86 && PCI
>  depends on !PREEMPT_RT
> +   depends on (BACKLIGHT_CLASS_DEVICE && ACPI) || 
> (BACKLIGHT_CLASS_DEVICE=n && ACPI=n)
>  select INTEL_GTT if X86
>  select INTERVAL_TREE
>  # we need shmfs for the swappable backing store, and in particular
> @@ -21,7 +22,6 @@ config DRM_I915
>  select IRQ_WORK
>  # i915 depends on ACPI_VIDEO when ACPI is enabled
>  # but for select to work, need to select ACPI_VIDEO's 
> dependencies, ick
> -   select BACKLIGHT_CLASS_DEVICE if ACPI
>  select INPUT if ACPI
>  select X86_PLATFORM_DEVICES if ACPI
>  select ACPI_WMI if ACPI
>
> I get:
> drivers/gpu/drm/i915/Kconfig:2:error: recursive dependency detected!
> drivers/gpu/drm/i915/Kconfig:2: symbol DRM_I915 depends on 
> BACKLIGHT_CLASS_DEVICE
> drivers/video/backlight/Kconfig:143:symbol BACKLIGHT_CLASS_DEVICE is 
> selected by DRM_FSL_DCU
> drivers/gpu/drm/fsl-dcu/Kconfig:2:  symbol DRM_FSL_DCU depends on 
> COMMON_CLK
> drivers/clk/Kconfig:21: symbol COMMON_CLK is selected by X86_INTEL_QUARK
> arch/x86/Kconfig:633:   symbol X86_INTEL_QUARK depends on 
> X86_PLATFORM_DEVICES
> drivers/platform/x86/Kconfig:6: symbol X86_PLATFORM_DEVICES is selected 
> by DRM_I915
>
>
> Those dependencies are really cumbersome :/.

Yes. They could be fixed, though [1]. IS_REACHABLE() is just band-aid.

BR,
Jani.


[1] 
https://lore.kernel.org/r/1413580403-16225-1-git-send-email-jani.nik...@intel.com

>
>> Documentation/kbuild/kconfig-language.rst:
>> 
>>Note:
>>  select should be used with care. select will force
>>  a symbol to a value without visiting the dependencies.
>>  By abusing select you are able to select a symbol FOO even
>>  if FOO depends on BAR that is not set.
>>  In general use select only for non-visible symbols
>>  (no prompts anywhere) and for symbols with no dependencies.
>>  That will limit the usefulness but on the other hand avoid
>>  the illegal configurations all over.
>> 
>> 
>> BR,
>> Jani.
>> 
>>>
>>> Cc: Jani Nikula 
>>> Cc: Joonas Lahtinen 
>>> Cc: Rodrigo Vivi 
>>> Cc: Tvrtko Ursulin 
>>> Cc: David Airlie 
>>> Cc: Daniel Vetter 
>>> Cc: intel-...@lists.freedesktop.org
>>> Cc: dri-devel@lists.freedesktop.org
>>> Reported-by: Martin Liška 
>>> Signed-off-by: Jiri Slaby (SUSE) 
>>> ---
>>>   drivers/gpu/drm/i915/display/intel_backlight.c | 2 +-
>>>   drivers/gpu/drm/i915/display/intel_backlight.h | 2 +-
>>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c 
>>> b/drivers/gpu/drm/i915/display/intel_backlight.c
>>> index beba39a38c87..c1ba68796b6d 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_backlight.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_backlight.c
>>> @@ -825,7 +825,7 @@ void intel_backlight_enable(const struct 
>>> intel_crtc_state *crtc_state,
>>> mutex_unlock(&dev_priv->display.backlight.lock);
>>>   }
>>>   
>>> -#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
>>> +#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
>>>   static u32 intel_panel_get_backlight(struct intel_connector *connector)
>>>   {
>>> struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>>> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.h 
>>> b/drivers/gpu/drm/i915/display/intel_backlight.h
>>> index 339643f63897..207fe1c613d8 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_backlight.h
>>> +++ b/dr

Re: [PATCH 1/2] drm/i915/display: fix randconfig build

2022-10-05 Thread Jiri Slaby


On 04. 10. 22, 12:52, Jani Nikula wrote:

On Tue, 04 Oct 2022, "Jiri Slaby (SUSE)"  wrote:

When DRM_I915=y and BACKLIGHT_CLASS_DEVICE=m, the build fails:
ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function 
`intel_backlight_device_register':
intel_backlight.c:(.text+0x5587): undefined reference to 
`backlight_device_get_by_name'

ld: drivers/gpu/drm/i915/display/intel_backlight.o: in function 
`intel_backlight_device_unregister':
intel_backlight.c:(.text+0x576e): undefined reference to 
`backlight_device_unregister'

To fix this, use IS_REACHABLE(), not IS_ENABLED() in backlight. That is,
with the above config, backlight support is disabled.


So I don't want this. I'll take a patch that fixes the dependencies to
block DRM_I915=y and BACKLIGHT_CLASS_DEVICE=m. Nobody wants that combo,
and IMO using IS_REACHABLE() is a workaround to hide a broken config
under the carpet.

The right thing to do is

config DRM_I915
depends on BACKLIGHT_CLASS_DEVICE || BACKLIGHT_CLASS_DEVICE=n.

We're selecting BACKLIGHT_CLASS_DEVICE because almost everyone else is
too, and a combo of selecting and depending leads to circular
dependencies. But depending is the right fix.


I'm not sure what should I do now. If I do:
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -4,6 +4,7 @@ config DRM_I915
depends on DRM
depends on X86 && PCI
depends on !PREEMPT_RT
+   depends on (BACKLIGHT_CLASS_DEVICE && ACPI) || 
(BACKLIGHT_CLASS_DEVICE=n && ACPI=n)

select INTEL_GTT if X86
select INTERVAL_TREE
# we need shmfs for the swappable backing store, and in particular
@@ -21,7 +22,6 @@ config DRM_I915
select IRQ_WORK
# i915 depends on ACPI_VIDEO when ACPI is enabled
# but for select to work, need to select ACPI_VIDEO's 
dependencies, ick

-   select BACKLIGHT_CLASS_DEVICE if ACPI
select INPUT if ACPI
select X86_PLATFORM_DEVICES if ACPI
select ACPI_WMI if ACPI

I get:
drivers/gpu/drm/i915/Kconfig:2:error: recursive dependency detected!
drivers/gpu/drm/i915/Kconfig:2: symbol DRM_I915 depends on 
BACKLIGHT_CLASS_DEVICE
drivers/video/backlight/Kconfig:143:symbol BACKLIGHT_CLASS_DEVICE is 
selected by DRM_FSL_DCU
drivers/gpu/drm/fsl-dcu/Kconfig:2:  symbol DRM_FSL_DCU depends on 
COMMON_CLK

drivers/clk/Kconfig:21: symbol COMMON_CLK is selected by X86_INTEL_QUARK
arch/x86/Kconfig:633:   symbol X86_INTEL_QUARK depends on 
X86_PLATFORM_DEVICES
drivers/platform/x86/Kconfig:6: symbol X86_PLATFORM_DEVICES is selected 
by DRM_I915



Those dependencies are really cumbersome :/.


Documentation/kbuild/kconfig-language.rst:

   Note:
select should be used with care. select will force
a symbol to a value without visiting the dependencies.
By abusing select you are able to select a symbol FOO even
if FOO depends on BAR that is not set.
In general use select only for non-visible symbols
(no prompts anywhere) and for symbols with no dependencies.
That will limit the usefulness but on the other hand avoid
the illegal configurations all over.


BR,
Jani.



Cc: Jani Nikula 
Cc: Joonas Lahtinen 
Cc: Rodrigo Vivi 
Cc: Tvrtko Ursulin 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: intel-...@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Reported-by: Martin Liška 
Signed-off-by: Jiri Slaby (SUSE) 
---
  drivers/gpu/drm/i915/display/intel_backlight.c | 2 +-
  drivers/gpu/drm/i915/display/intel_backlight.h | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c 
b/drivers/gpu/drm/i915/display/intel_backlight.c
index beba39a38c87..c1ba68796b6d 100644
--- a/drivers/gpu/drm/i915/display/intel_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_backlight.c
@@ -825,7 +825,7 @@ void intel_backlight_enable(const struct intel_crtc_state 
*crtc_state,
mutex_unlock(&dev_priv->display.backlight.lock);
  }
  
-#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)

+#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
  static u32 intel_panel_get_backlight(struct intel_connector *connector)
  {
struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
diff --git a/drivers/gpu/drm/i915/display/intel_backlight.h 
b/drivers/gpu/drm/i915/display/intel_backlight.h
index 339643f63897..207fe1c613d8 100644
--- a/drivers/gpu/drm/i915/display/intel_backlight.h
+++ b/drivers/gpu/drm/i915/display/intel_backlight.h
@@ -36,7 +36,7 @@ u32 intel_backlight_invert_pwm_level(struct intel_connector 
*connector, u32 leve
  u32 intel_backlight_level_to_pwm(struct intel_connector *connector, u32 
level);
  u32 intel_backlight_level_from_pwm(struct intel_connector *connector, u32 
val);
  
-#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)

+#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
  int intel_backlight_device_register(struct intel_connector *connector);
  void intel_backlight_device_unregister(struct intel_c

Re: [PATCH v2 0/2] drm/rockchip: dw_hdmi: Add 4k@30 support

2022-10-05 Thread Sascha Hauer

On Wed, Oct 05, 2022 at 12:51:57PM +0200, Dan Johansen wrote:
> 
> Den 05.10.2022 kl. 12.06 skrev Sascha Hauer:
> > On Wed, Sep 28, 2022 at 10:39:27AM +0200, Dan Johansen wrote:
> > > Den 28.09.2022 kl. 10.37 skrev Sascha Hauer:
> > > > On Tue, Sep 27, 2022 at 07:53:54PM +0200, Dan Johansen wrote:
> > > > > Den 26.09.2022 kl. 12.30 skrev Michael Riesch:
> > > > > > Hi Sascha,
> > > > > > 
> > > > > > On 9/26/22 10:04, Sascha Hauer wrote:
> > > > > > > This series adds support for 4k@30 to the rockchip HDMI 
> > > > > > > controller. This
> > > > > > > has been tested on a rk3568 rock3a board. It should be possible 
> > > > > > > to add
> > > > > > > 4k@60 support the same way, but it doesn't work for me, so let's 
> > > > > > > add
> > > > > > > 4k@30 as a first step.
> > > > > > >   
> > > > > > >  Sascha
> > > > > > > 
> > > > > > > Changes since v1:
> > > > > > > - Allow non standard clock rates only on Synopsys phy as 
> > > > > > > suggested by
> > > > > > >  Robin Murphy
> > > > > > > 
> > > > > > > Sascha Hauer (2):
> > > > > > >  drm/rockchip: dw_hdmi: relax mode_valid hook
> > > > > > >  drm/rockchip: dw_hdmi: Add support for 4k@30 resolution
> > > > > > > 
> > > > > > > drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 34 
> > > > > > > -
> > > > > > > 1 file changed, 27 insertions(+), 7 deletions(-)
> > > > > > Thanks for the v2! On a RK3568 EVB1 with a HP 27f 4k monitor
> > > > > > 
> > > > > > Tested-by: Michael Riesch 
> > > > > Sadly this still doesn't give my display out on my 2k monitor. Not 
> > > > > even just
> > > > > 1080p picture like the old current implementation does.
> > > > By "like the old current implementation" you mean that this patchset
> > > > introduces a regression for you?
> > > Yes. What currently in the kernel at least shows as 1080p on my 2K 
> > > monitor,
> > > while this patchset turns off the screen.
> > Which SoC are you testing this on? I assume RK3568, right? Which patch
> > introduces that regression, the first or the second one?
> I tested on the Odroid M, which is rk3568.
> I have only applied them both, as I was under the impression that both are
> needed for the 4k support.

Yes, both I needed, but I am interested which one introduces the
regression as I can't reproduce it.

Sascha

-- 
Pengutronix e.K.   | |
Steuerwalder Str. 21   | http://www.pengutronix.de/  |
31137 Hildesheim, Germany  | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

Re: [PATCH v2 0/2] drm/rockchip: dw_hdmi: Add 4k@30 support

2022-10-05 Thread Dan Johansen




Den 05.10.2022 kl. 12.06 skrev Sascha Hauer:

On Wed, Sep 28, 2022 at 10:39:27AM +0200, Dan Johansen wrote:

Den 28.09.2022 kl. 10.37 skrev Sascha Hauer:

On Tue, Sep 27, 2022 at 07:53:54PM +0200, Dan Johansen wrote:

Den 26.09.2022 kl. 12.30 skrev Michael Riesch:

Hi Sascha,

On 9/26/22 10:04, Sascha Hauer wrote:

This series adds support for 4k@30 to the rockchip HDMI controller. This
has been tested on a rk3568 rock3a board. It should be possible to add
4k@60 support the same way, but it doesn't work for me, so let's add
4k@30 as a first step.

 Sascha

Changes since v1:
- Allow non standard clock rates only on Synopsys phy as suggested by
 Robin Murphy

Sascha Hauer (2):
 drm/rockchip: dw_hdmi: relax mode_valid hook
 drm/rockchip: dw_hdmi: Add support for 4k@30 resolution

drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 34 -
1 file changed, 27 insertions(+), 7 deletions(-)

Thanks for the v2! On a RK3568 EVB1 with a HP 27f 4k monitor

Tested-by: Michael Riesch 

Sadly this still doesn't give my display out on my 2k monitor. Not even just
1080p picture like the old current implementation does.

By "like the old current implementation" you mean that this patchset
introduces a regression for you?

Yes. What currently in the kernel at least shows as 1080p on my 2K monitor,
while this patchset turns off the screen.

Which SoC are you testing this on? I assume RK3568, right? Which patch
introduces that regression, the first or the second one?

I tested on the Odroid M, which is rk3568.
I have only applied them both, as I was under the impression that both 
are needed for the 4k support.


Sascha


--
Kind regards
*Dan Johansen*
Project lead of the *Manjaro ARM* project
Manjaro-ARM

[RFC 2/2] drm/i915: iterate intel_connectors only

The drm_connectors that are embedded in writeback connectors won't be
embedded in intel_connectors, so we can't assume being able to convert a
drm_connector to intel_connector when iterating all the connectors we
have. Use the drm connector list filtering to skip writeback connectors.

Note: We could also wrap the begin/end calls to intel specific macros to
hide the boilerplate.

Cc: Arun R Murthy 
Cc: Suraj Kandpal 
Cc: Ville Syrjälä 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/display/intel_display.c   |  3 ++-
 drivers/gpu/drm/i915/display/intel_display_types.h |  7 +++
 drivers/gpu/drm/i915/display/intel_dp.c|  6 --
 drivers/gpu/drm/i915/display/intel_dp_mst.c|  3 ++-
 drivers/gpu/drm/i915/display/intel_hdcp.c  |  3 ++-
 drivers/gpu/drm/i915/display/intel_hotplug.c   | 12 
 drivers/gpu/drm/i915/display/intel_modeset_setup.c |  6 --
 drivers/gpu/drm/i915/display/intel_opregion.c  |  9 ++---
 8 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 8c3bd9ba0d74..892a86c86fb4 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8960,7 +8960,8 @@ static void intel_hpd_poll_fini(struct drm_i915_private 
*i915)
struct drm_connector_list_iter conn_iter;
 
/* Kill all the work that may have been queued by hpd. */
-   drm_connector_list_iter_begin(&i915->drm, &conn_iter);
+   drm_connector_list_iter_filter_begin(&i915->drm, &conn_iter,
+is_intel_connector, NULL);
for_each_intel_connector_iter(connector, &conn_iter) {
if (connector->modeset_retry_work.func)
cancel_work_sync(&connector->modeset_retry_work);
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h 
b/drivers/gpu/drm/i915/display/intel_display_types.h
index e2b853e9e51d..49a5a1ddc4df 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -612,6 +612,13 @@ struct intel_connector {
struct intel_hdcp hdcp;
 };
 
+static inline bool is_intel_connector(const struct drm_connector *connector,
+ void *unused_filter_context)
+{
+   /* writeback connectors aren't embedded in intel_connector */
+   return connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK;
+}
+
 struct intel_digital_connector_state {
struct drm_connector_state base;
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
b/drivers/gpu/drm/i915/display/intel_dp.c
index 70b06806ec0d..4cf934c51290 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4044,7 +4044,8 @@ static int intel_dp_prep_link_retrain(struct intel_dp 
*intel_dp,
if (!intel_dp_needs_link_retrain(intel_dp))
return 0;
 
-   drm_connector_list_iter_begin(&i915->drm, &conn_iter);
+   drm_connector_list_iter_filter_begin(&i915->drm, &conn_iter,
+is_intel_connector, NULL);
for_each_intel_connector_iter(connector, &conn_iter) {
struct drm_connector_state *conn_state =
connector->base.state;
@@ -4173,7 +4174,8 @@ static int intel_dp_prep_phy_test(struct intel_dp 
*intel_dp,
 
*pipe_mask = 0;
 
-   drm_connector_list_iter_begin(&i915->drm, &conn_iter);
+   drm_connector_list_iter_filter_begin(&i915->drm, &conn_iter,
+is_intel_connector, NULL);
for_each_intel_connector_iter(connector, &conn_iter) {
struct drm_connector_state *conn_state =
connector->base.state;
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c 
b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index cd4e61026d98..e4f98cee8d81 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -272,7 +272,8 @@ intel_dp_mst_atomic_master_trans_check(struct 
intel_connector *connector,
if (!intel_connector_needs_modeset(state, &connector->base))
return 0;
 
-   drm_connector_list_iter_begin(&dev_priv->drm, &connector_list_iter);
+   drm_connector_list_iter_filter_begin(&dev_priv->drm, 
&connector_list_iter,
+is_intel_connector, NULL);
for_each_intel_connector_iter(connector_iter, &connector_list_iter) {
struct intel_digital_connector_state *conn_iter_state;
struct intel_crtc_state *crtc_state;
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c 
b/drivers/gpu/drm/i915/display/intel_hdcp.c
index 6406fd487ee5..36167e42a537 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -83,7 +83,8 @@ i

[RFC 1/2] drm/connector: add connector list iteration with filtering

Add new function drm_connector_list_iter_filter_begin() to initialize
connector list iterator with a filter function. Subsequent iteration on
the list will only return connectors on which the filter function
returns true.

Cc: Arun R Murthy 
Cc: Suraj Kandpal 
Cc: Ville Syrjälä 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/drm_connector.c | 57 ++---
 include/drm/drm_connector.h |  9 ++
 2 files changed, 54 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index e3142c8142b3..d54b4b54cecb 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -762,6 +762,29 @@ static struct lockdep_map connector_list_iter_dep_map = {
 };
 #endif
 
+/**
+ * drm_connector_list_iter_filter_begin - initialize a connector_list iterator 
with filter
+ * @dev: DRM device
+ * @iter: connector_list iterator
+ * @filter: connector filter function
+ * @filter_context: context to be passed to the filter function
+ *
+ * Same as drm_connector_list_iter_begin(), but sets up the iterator to only
+ * return connectors where filter(connector) returns true.
+ */
+void drm_connector_list_iter_filter_begin(struct drm_device *dev,
+ struct drm_connector_list_iter *iter,
+ drm_connector_list_iter_filter_fn 
filter,
+ void *filter_context)
+{
+   iter->dev = dev;
+   iter->conn = NULL;
+   iter->filter = filter;
+   iter->filter_context = filter_context;
+   lock_acquire_shared_recursive(&connector_list_iter_dep_map, 0, 1, NULL, 
_RET_IP_);
+}
+EXPORT_SYMBOL(drm_connector_list_iter_filter_begin);
+
 /**
  * drm_connector_list_iter_begin - initialize a connector_list iterator
  * @dev: DRM device
@@ -775,9 +798,7 @@ static struct lockdep_map connector_list_iter_dep_map = {
 void drm_connector_list_iter_begin(struct drm_device *dev,
   struct drm_connector_list_iter *iter)
 {
-   iter->dev = dev;
-   iter->conn = NULL;
-   lock_acquire_shared_recursive(&connector_list_iter_dep_map, 0, 1, NULL, 
_RET_IP_);
+   drm_connector_list_iter_filter_begin(dev, iter, NULL, NULL);
 }
 EXPORT_SYMBOL(drm_connector_list_iter_begin);
 
@@ -800,15 +821,8 @@ __drm_connector_put_safe(struct drm_connector *conn)
schedule_work(&config->connector_free_work);
 }
 
-/**
- * drm_connector_list_iter_next - return next connector
- * @iter: connector_list iterator
- *
- * Returns: the next connector for @iter, or NULL when the list walk has
- * completed.
- */
-struct drm_connector *
-drm_connector_list_iter_next(struct drm_connector_list_iter *iter)
+static struct drm_connector *
+__drm_connector_list_iter_next(struct drm_connector_list_iter *iter)
 {
struct drm_connector *old_conn = iter->conn;
struct drm_mode_config *config = &iter->dev->mode_config;
@@ -836,6 +850,25 @@ drm_connector_list_iter_next(struct 
drm_connector_list_iter *iter)
 
return iter->conn;
 }
+
+/**
+ * drm_connector_list_iter_next - return next connector
+ * @iter: connector_list iterator
+ *
+ * Returns: the next connector for @iter, or NULL when the list walk has
+ * completed.
+ */
+struct drm_connector *
+drm_connector_list_iter_next(struct drm_connector_list_iter *iter)
+{
+   struct drm_connector *connector;
+
+   while ((connector = __drm_connector_list_iter_next(iter)) &&
+  iter->filter && !iter->filter(connector, iter->filter_context))
+   ;
+
+   return connector;
+}
 EXPORT_SYMBOL(drm_connector_list_iter_next);
 
 /**
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 56aee949c6fa..497b98197d3a 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1868,6 +1868,9 @@ struct drm_tile_group *drm_mode_get_tile_group(struct 
drm_device *dev,
 void drm_mode_put_tile_group(struct drm_device *dev,
 struct drm_tile_group *tg);
 
+typedef bool (*drm_connector_list_iter_filter_fn)(const struct drm_connector 
*connector,
+ void *filter_context);
+
 /**
  * struct drm_connector_list_iter - connector_list iterator
  *
@@ -1886,10 +1889,16 @@ struct drm_connector_list_iter {
 /* private: */
struct drm_device *dev;
struct drm_connector *conn;
+   drm_connector_list_iter_filter_fn filter;
+   void *filter_context;
 };
 
 void drm_connector_list_iter_begin(struct drm_device *dev,
   struct drm_connector_list_iter *iter);
+void drm_connector_list_iter_filter_begin(struct drm_device *dev,
+ struct drm_connector_list_iter *iter,
+ drm_connector_list_iter_filter_fn 
filter,
+ void *filter_context);
 struct drm_connector *
 drm_connector

[RFC 0/2] drm/connector: connector iterator with filtering

Currently i915 assumes all drm_connectors it encounters are embedded in
intel_connectors that i915 allocated. The drm_writeback_connector forces
a design where this is not the case; we can't provide our own connector,
and writeback embeds the drm_connector it initializes itself.

To use drm writeback, none of the i915 connector iteration could assume
the drm connector is embedded in intel_connector. Checking this is
tedious, and would require an intermediate step with
drm_connector. Here's an idea I came up with; filtering at the drm
connector iterator level with a caller supplied function. Not too much
code, and could be used for other things as well.

Mind you, we'd still much rather modify drm writeback to allow passing
the connector i915 allocated, instead of the current midlayer design
that forces drivers to a certain model. Working around this is a bunch
of error prone and tedious code that we really could do without.


BR,
Jani.


Cc: Arun R Murthy 
Cc: Dave Airlie 
Cc: Laurent Pinchart 
Cc: Suraj Kandpal 
Cc: Ville Syrjälä 

Jani Nikula (2):
  drm/connector: add connector list iteration with filtering
  drm/i915: iterate intel_connectors only

 drivers/gpu/drm/drm_connector.c   | 57 +++
 drivers/gpu/drm/i915/display/intel_display.c  |  3 +-
 .../drm/i915/display/intel_display_types.h|  7 +++
 drivers/gpu/drm/i915/display/intel_dp.c   |  6 +-
 drivers/gpu/drm/i915/display/intel_dp_mst.c   |  3 +-
 drivers/gpu/drm/i915/display/intel_hdcp.c |  3 +-
 drivers/gpu/drm/i915/display/intel_hotplug.c  | 12 ++--
 .../drm/i915/display/intel_modeset_setup.c|  6 +-
 drivers/gpu/drm/i915/display/intel_opregion.c |  9 ++-
 include/drm/drm_connector.h   |  9 +++
 10 files changed, 89 insertions(+), 26 deletions(-)

-- 
2.34.1

Re: [PATCH v2 0/2] drm/rockchip: dw_hdmi: Add 4k@30 support

2022-10-05 Thread Sascha Hauer

On Wed, Sep 28, 2022 at 10:39:27AM +0200, Dan Johansen wrote:
> 
> Den 28.09.2022 kl. 10.37 skrev Sascha Hauer:
> > On Tue, Sep 27, 2022 at 07:53:54PM +0200, Dan Johansen wrote:
> > > Den 26.09.2022 kl. 12.30 skrev Michael Riesch:
> > > > Hi Sascha,
> > > > 
> > > > On 9/26/22 10:04, Sascha Hauer wrote:
> > > > > This series adds support for 4k@30 to the rockchip HDMI controller. 
> > > > > This
> > > > > has been tested on a rk3568 rock3a board. It should be possible to add
> > > > > 4k@60 support the same way, but it doesn't work for me, so let's add
> > > > > 4k@30 as a first step.
> > > > >   
> > > > >  Sascha
> > > > > 
> > > > > Changes since v1:
> > > > > - Allow non standard clock rates only on Synopsys phy as suggested by
> > > > > Robin Murphy
> > > > > 
> > > > > Sascha Hauer (2):
> > > > > drm/rockchip: dw_hdmi: relax mode_valid hook
> > > > > drm/rockchip: dw_hdmi: Add support for 4k@30 resolution
> > > > > 
> > > > >drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 34 
> > > > > -
> > > > >1 file changed, 27 insertions(+), 7 deletions(-)
> > > > Thanks for the v2! On a RK3568 EVB1 with a HP 27f 4k monitor
> > > > 
> > > > Tested-by: Michael Riesch 
> > > Sadly this still doesn't give my display out on my 2k monitor. Not even 
> > > just
> > > 1080p picture like the old current implementation does.
> > By "like the old current implementation" you mean that this patchset
> > introduces a regression for you?
> Yes. What currently in the kernel at least shows as 1080p on my 2K monitor,
> while this patchset turns off the screen.

Which SoC are you testing this on? I assume RK3568, right? Which patch
introduces that regression, the first or the second one?

Sascha

-- 
Pengutronix e.K.   | |
Steuerwalder Str. 21   | http://www.pengutronix.de/  |
31137 Hildesheim, Germany  | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

[RFC PATCH 0/1] drm/bridge: ti-sn65dsi83: Fix LVDS panel overclocking

2022-10-05 Thread estl

From: Eberhard Stoll 

I'm currently working on a board with stm32mp157 SOC which contains
a synposys LTDC to DSI bridge (drm/stm/dw_mipi_dsi-stm.c). In this
configuration i observe a overclocking of the LVDS pixel clock.

The reason for this is, that the synopsys bridge increases the DSI
lane clock by 20% for burst mode. On the other hand the sn65dsi83
chip uses the DSI lane clock to generate LVDS pixel clock. So in
this configuration the LVDS pixel clock is 20% higher as the panel
configuration pretends.

Removing burst mode solves this issue. It also seems sensible to
me, that a fixed divider to generate the pixel clock out of the DSI
lane clock is not compatible with burst mode.

But i'm not sure if i'm missing someting in the big picture and
whether this issue should be solved somewhere else.

Eberhard Stoll (1):
  drm/bridge: ti-sn65dsi83: Remove burst mode

 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--
2.25.1

[RFC PATCH 1/1] drm/bridge: ti-sn65dsi83: Remove burst mode

2022-10-05 Thread estl

From: Eberhard Stoll 

Remove LVDS panel overclocking for some configurations by disabling
burst mode for this chip

With burst mode enabled, some DSI controllers increase their DSI lane
clock beyond the clock which is needed to transfer all pixel data.

But this driver operates with a pixel clock derived from the DSI lane
clock by a fixed prescaler. So, every increase of the DSI clock also
increases the LVDS pixel clock. In this case, the LVDS pixel clock is
overclocked.

This is the case e.g. for synopsys stm DSI bridge
(drm/stm/dw_mipi_dsi-stm.c). With burst mode the DSI clock is
increased by 20% and therefore also for the LVDS panel.

Signed-off-by: Eberhard Stoll 
---
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c 
b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index c901c0e1a3b0..ffc39208536e 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -272,7 +272,7 @@ static int sn65dsi83_attach(struct drm_bridge *bridge,

dsi->lanes = ctx->dsi_lanes;
dsi->format = MIPI_DSI_FMT_RGB888;
-   dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST;
+   dsi->mode_flags = MIPI_DSI_MODE_VIDEO;

ret = mipi_dsi_attach(dsi);
if (ret < 0) {
--
2.25.1

Re: [PATCH v1 17/17] drm/mediatek: Add mt8195-dpi support to drm_drv

2022-10-05 Thread Guillaume Ranquet

On Tue, 04 Oct 2022 17:05, Krzysztof Kozlowski
 wrote:
>On 04/10/2022 13:55, Guillaume Ranquet wrote:
>>> No. You said what the code is doing. I think I understand this. You
>>> still do not need more compatibles. Your sentence did not clarify it
>>> because it did not answer at all to question "why". Why do you need it?
>>>
>>> Sorry, the change looks not correct.
>>>
>>> Best regards,
>>> Krzysztof
>>>
>>
>> I need a new compatible to adress the specifics of mt8195 in the mtk_dpi 
>> driver,
>> the change is in this series with:
>> [PATCH v1 16/17] drm/mediatek: dpi: Add mt8195 hdmi to DPI driver [1]
>
>But you do not have specifics of mt8195. I wrote it in the beginning.
>
>>
>> I then need to add that compatible to the "list" here in mtk_drm_drv.
>
>No, you do not... I checked the driver and there is no single need... or
>convince me you need.
>
>> I don't see a way around this unless I rewrite the way mtk_drm_drv works?
>
>Why rewrite? You have all compatibles in place.
>
>>
>> Maybe if I declare a new compatible that is generic to all mediatek
>> dpi variants?
>
>You were asked to use fallback. Don't create some fake fallbacks. Use
>existing ones.
>
>> and have all the dts specify the node with both the generic dpi and
>> the specific compatible?
>>
>> dpi@xxx {
>>  compatible = "mediatek,dpi", "mediatek,mt8195-dpi";
>
>I don't know what's this but certainly looks odd. Some wild-card
>compatible in front (not fallback) and none are documented.
>
>>  ...
>> }
>>
>> Then I can "collapse" all the dpi related nodes in mtk_drm_drv under
>> "mediatek,dpi" ?
>>
>> I guess would have to do the change for all other components that are needed 
>> in
>> mtk_drm_drv (mmsys, aal, ccor, color, dither, dsc, gamma, mutex...).
>>
>> That's the only trivial way I can think of implementing this with the
>> current status
>> of the mtk_drm stack.
>>
>> Do you have any other ideas in mind?
>
>Use fallback of compatible device. That's the common pattern.
>Everywhere, Mediatek as well.
>

I'm unsure about what a "fallback of compatible device" is.
But from what I can gather, you'd have me write the dts as:

dpi@xxx {
compatible = "mediatek,mt8195-dpi", "mediatek,mt2701-dpi";
...
}

so that the mtk_dpi driver will use the specific mt8195 dpi config and
the mtk_drm driver will fallback to mt2701 to find the compatible it needs.

Would you like me to use this pattern for all the other compatibles declared
in the mtk_ddp_comp_ids array in separate patches?

Thx,
Guillaume.

>Best regards,
>Krzysztof
>

[PATCH v7 6/6] arm64: dts: qcom: sc7280: Add Reset support for gpu

Add support for Reset using GPUCC driver for GPU. This helps to ensure
that GPU state is reset by making sure that CX head switch is collapsed.

Signed-off-by: Akhil P Oommen 
---

(no changes since v1)

 arch/arm64/boot/dts/qcom/sc7280.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi 
b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 2125803..3e559b3 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2535,6 +2535,9 @@
nvmem-cells = <&gpu_speed_bin>;
nvmem-cell-names = "speed_bin";
 
+   resets = <&gpucc GPU_CX_COLLAPSE>;
+   reset-names = "cx_collapse";
+
gpu_opp_table: opp-table {
compatible = "operating-points-v2";
 
-- 
2.7.4

[PATCH v7 5/6] dt-bindings: drm/msm/gpu: Add optional resets

Add an optional reference to GPUCC reset which can be used to ensure cx
gdsc collapse during gpu recovery.

Signed-off-by: Akhil P Oommen 
Acked-by: Rob Herring 
Acked-by: Krzysztof Kozlowski 
---

(no changes since v5)

Changes in v5:
- Nit: Remove a duplicate blank line (Krzysztof)

Changes in v4:
- New patch in v4

 Documentation/devicetree/bindings/display/msm/gpu.yaml | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/msm/gpu.yaml 
b/Documentation/devicetree/bindings/display/msm/gpu.yaml
index 3397bc3..408ed97 100644
--- a/Documentation/devicetree/bindings/display/msm/gpu.yaml
+++ b/Documentation/devicetree/bindings/display/msm/gpu.yaml
@@ -109,6 +109,12 @@ properties:
   For GMU attached devices a phandle to the GMU device that will
   control the power for the GPU.
 
+  resets:
+maxItems: 1
+
+  reset-names:
+items:
+  - const: cx_collapse
 
 required:
   - compatible
-- 
2.7.4

[PATCH v7 4/6] clk: qcom: gpucc-sc7280: Add cx collapse reset support

Allow a consumer driver to poll for cx gdsc collapse through Reset
framework.

Signed-off-by: Akhil P Oommen 
Reviewed-by: Dmitry Baryshkov 
---

(no changes since v3)

Changes in v3:
- Convert 'struct qcom_reset_ops cx_gdsc_reset' to 'static const' (Krzysztof)

Changes in v2:
- Minor update to use the updated custom reset ops implementation

 drivers/clk/qcom/gpucc-sc7280.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/clk/qcom/gpucc-sc7280.c b/drivers/clk/qcom/gpucc-sc7280.c
index 9a832f2..fece3f4 100644
--- a/drivers/clk/qcom/gpucc-sc7280.c
+++ b/drivers/clk/qcom/gpucc-sc7280.c
@@ -433,12 +433,22 @@ static const struct regmap_config 
gpu_cc_sc7280_regmap_config = {
.fast_io = true,
 };
 
+static const struct qcom_reset_ops cx_gdsc_reset = {
+   .reset = gdsc_wait_for_collapse,
+};
+
+static const struct qcom_reset_map gpucc_sc7280_resets[] = {
+   [GPU_CX_COLLAPSE] = { .ops = &cx_gdsc_reset, .priv = &cx_gdsc },
+};
+
 static const struct qcom_cc_desc gpu_cc_sc7280_desc = {
.config = &gpu_cc_sc7280_regmap_config,
.clks = gpu_cc_sc7280_clocks,
.num_clks = ARRAY_SIZE(gpu_cc_sc7280_clocks),
.gdscs = gpu_cc_sc7180_gdscs,
.num_gdscs = ARRAY_SIZE(gpu_cc_sc7180_gdscs),
+   .resets = gpucc_sc7280_resets,
+   .num_resets = ARRAY_SIZE(gpucc_sc7280_resets),
 };
 
 static const struct of_device_id gpu_cc_sc7280_match_table[] = {
-- 
2.7.4

[PATCH v7 3/6] clk: qcom: gdsc: Add a reset op to poll gdsc collapse

Add a reset op compatible function to poll for gdsc collapse. This is
required because:
  1. We don't wait for it to turn OFF at hardware for VOTABLE GDSCs.
  2. There is no way for client drivers (eg. gpu driver) to do
  put-with-wait for these gdscs which is required in some scenarios
  (eg. GPU recovery).

Signed-off-by: Akhil P Oommen 
Reviewed-by: Dmitry Baryshkov 
---

Changes in v7:
- Update commit message (Bjorn)

Changes in v2:
- Minor update to function prototype

 drivers/clk/qcom/gdsc.c | 23 +++
 drivers/clk/qcom/gdsc.h |  7 +++
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index 7cf5e13..ccef742 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include "gdsc.h"
+#include "reset.h"
 
 #define PWR_ON_MASKBIT(31)
 #define EN_REST_WAIT_MASK  GENMASK_ULL(23, 20)
@@ -116,7 +117,8 @@ static int gdsc_hwctrl(struct gdsc *sc, bool en)
return regmap_update_bits(sc->regmap, sc->gdscr, HW_CONTROL_MASK, val);
 }
 
-static int gdsc_poll_status(struct gdsc *sc, enum gdsc_status status)
+static int gdsc_poll_status(struct gdsc *sc, enum gdsc_status status,
+   s64 timeout_us, unsigned int interval_ms)
 {
ktime_t start;
 
@@ -124,7 +126,9 @@ static int gdsc_poll_status(struct gdsc *sc, enum 
gdsc_status status)
do {
if (gdsc_check_status(sc, status))
return 0;
-   } while (ktime_us_delta(ktime_get(), start) < TIMEOUT_US);
+   if (interval_ms)
+   msleep(interval_ms);
+   } while (ktime_us_delta(ktime_get(), start) < timeout_us);
 
if (gdsc_check_status(sc, status))
return 0;
@@ -189,7 +193,7 @@ static int gdsc_toggle_logic(struct gdsc *sc, enum 
gdsc_status status)
udelay(1);
}
 
-   ret = gdsc_poll_status(sc, status);
+   ret = gdsc_poll_status(sc, status, TIMEOUT_US, 0);
WARN(ret, "%s status stuck at 'o%s'", sc->pd.name, status ? "ff" : "n");
 
if (!ret && status == GDSC_OFF && sc->rsupply) {
@@ -360,7 +364,7 @@ static int _gdsc_disable(struct gdsc *sc)
 */
udelay(1);
 
-   ret = gdsc_poll_status(sc, GDSC_ON);
+   ret = gdsc_poll_status(sc, GDSC_ON, TIMEOUT_US, 0);
if (ret)
return ret;
}
@@ -608,3 +612,14 @@ int gdsc_gx_do_nothing_enable(struct generic_pm_domain 
*domain)
return 0;
 }
 EXPORT_SYMBOL_GPL(gdsc_gx_do_nothing_enable);
+
+int gdsc_wait_for_collapse(void *priv)
+{
+   struct gdsc *sc = priv;
+   int ret;
+
+   ret = gdsc_poll_status(sc, GDSC_OFF, 50, 5);
+   WARN(ret, "%s status stuck at 'on'", sc->pd.name);
+   return ret;
+}
+EXPORT_SYMBOL_GPL(gdsc_wait_for_collapse);
diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
index 981a12c..5395f69 100644
--- a/drivers/clk/qcom/gdsc.h
+++ b/drivers/clk/qcom/gdsc.h
@@ -12,6 +12,7 @@
 struct regmap;
 struct regulator;
 struct reset_controller_dev;
+struct qcom_reset_map;
 
 /**
  * struct gdsc - Globally Distributed Switch Controller
@@ -88,6 +89,7 @@ int gdsc_register(struct gdsc_desc *desc, struct 
reset_controller_dev *,
  struct regmap *);
 void gdsc_unregister(struct gdsc_desc *desc);
 int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain);
+int gdsc_wait_for_collapse(void *priv);
 #else
 static inline int gdsc_register(struct gdsc_desc *desc,
struct reset_controller_dev *rcdev,
@@ -97,5 +99,10 @@ static inline int gdsc_register(struct gdsc_desc *desc,
 }
 
 static inline void gdsc_unregister(struct gdsc_desc *desc) {};
+
+static int gdsc_wait_for_collapse(void *priv)
+{
+   return  -ENOSYS;
+}
 #endif /* CONFIG_QCOM_GDSC */
 #endif /* __QCOM_GDSC_H__ */
-- 
2.7.4

[PATCH v7 2/6] clk: qcom: Allow custom reset ops

Allow soc specific clk drivers to specify a custom reset operation. We
will use this in an upcoming patch to allow gpucc driver to specify a
differet reset operation for cx_gdsc.

Signed-off-by: Akhil P Oommen 
Reviewed-by: Dmitry Baryshkov 
---

(no changes since v3)

Changes in v3:
- Use pointer to const for "struct qcom_reset_ops" in qcom_reset_map (Krzysztof)

Changes in v2:
- Return error when a particular custom reset op is not implemented. (Dmitry)

 drivers/clk/qcom/reset.c | 27 ++-
 drivers/clk/qcom/reset.h |  8 
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/reset.c b/drivers/clk/qcom/reset.c
index 2a16adb..10ef71b 100644
--- a/drivers/clk/qcom/reset.c
+++ b/drivers/clk/qcom/reset.c
@@ -13,7 +13,20 @@
 
 static int qcom_reset(struct reset_controller_dev *rcdev, unsigned long id)
 {
-   struct qcom_reset_controller *rst = to_qcom_reset_controller(rcdev);
+   struct qcom_reset_controller *rst;
+   const struct qcom_reset_map *map;
+
+   rst = to_qcom_reset_controller(rcdev);
+   map = &rst->reset_map[id];
+
+   if (map->ops && map->ops->reset)
+   return map->ops->reset(map->priv);
+   /*
+* If custom ops is implemented but just not this callback, return
+* error
+*/
+   else if (map->ops)
+   return -EOPNOTSUPP;
 
rcdev->ops->assert(rcdev, id);
udelay(rst->reset_map[id].udelay ?: 1); /* use 1 us as default */
@@ -30,6 +43,12 @@ qcom_reset_assert(struct reset_controller_dev *rcdev, 
unsigned long id)
 
rst = to_qcom_reset_controller(rcdev);
map = &rst->reset_map[id];
+
+   if (map->ops && map->ops->assert)
+   return map->ops->assert(map->priv);
+   else if (map->ops)
+   return -EOPNOTSUPP;
+
mask = BIT(map->bit);
 
return regmap_update_bits(rst->regmap, map->reg, mask, mask);
@@ -44,6 +63,12 @@ qcom_reset_deassert(struct reset_controller_dev *rcdev, 
unsigned long id)
 
rst = to_qcom_reset_controller(rcdev);
map = &rst->reset_map[id];
+
+   if (map->ops && map->ops->deassert)
+   return map->ops->deassert(map->priv);
+   else if (map->ops)
+   return -EOPNOTSUPP;
+
mask = BIT(map->bit);
 
return regmap_update_bits(rst->regmap, map->reg, mask, 0);
diff --git a/drivers/clk/qcom/reset.h b/drivers/clk/qcom/reset.h
index b8c1135..a4d767b 100644
--- a/drivers/clk/qcom/reset.h
+++ b/drivers/clk/qcom/reset.h
@@ -8,10 +8,18 @@
 
 #include 
 
+struct qcom_reset_ops {
+   int (*reset)(void *priv);
+   int (*assert)(void *priv);
+   int (*deassert)(void *priv);
+};
+
 struct qcom_reset_map {
unsigned int reg;
u8 bit;
u8 udelay;
+   const struct qcom_reset_ops *ops;
+   void *priv;
 };
 
 struct regmap;
-- 
2.7.4

[PATCH v7 1/6] dt-bindings: clk: qcom: Support gpu cx gdsc reset

Add necessary definitions in gpucc bindings to ensure gpu cx gdsc collapse
through 'reset' framework for SC7280.

Signed-off-by: Akhil P Oommen 
Acked-by: Krzysztof Kozlowski 
---

(no changes since v1)

 include/dt-bindings/clock/qcom,gpucc-sc7280.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/dt-bindings/clock/qcom,gpucc-sc7280.h 
b/include/dt-bindings/clock/qcom,gpucc-sc7280.h
index 669b23b..843a31b 100644
--- a/include/dt-bindings/clock/qcom,gpucc-sc7280.h
+++ b/include/dt-bindings/clock/qcom,gpucc-sc7280.h
@@ -32,4 +32,7 @@
 #define GPU_CC_CX_GDSC 0
 #define GPU_CC_GX_GDSC 1
 
+/* GPU_CC reset IDs */
+#define GPU_CX_COLLAPSE0
+
 #endif
-- 
2.7.4

[PATCH v7 0/6] clk/qcom: Support gdsc collapse polling using 'reset' interface



Some clients like adreno gpu driver would like to ensure that its gdsc
is collapsed at hardware during a gpu reset sequence. This is because it
has a votable gdsc which could be ON due to a vote from another subsystem
like tz, hyp etc or due to an internal hardware signal. To allow
this, gpucc driver can expose an interface to the client driver using
reset framework. Using this the client driver can trigger a polling within
the gdsc driver.

This series is rebased on top of qcom/linux:for-next branch.

Related discussion: https://patchwork.freedesktop.org/patch/493144/

Changes in v7:
- Update commit message (Bjorn)
- Rebased on top of qcom/linux:for-next branch.

Changes in v6:
- No code changes in this version. Just captured the Acked-by tags

Changes in v5:
- Nit: Remove a duplicate blank line (Krzysztof)

Changes in v4:
- Update gpu dt-binding schema
- Typo fix in commit text

Changes in v3:
- Use pointer to const for "struct qcom_reset_ops" in qcom_reset_map (Krzysztof)

Changes in v2:
- Return error when a particular custom reset op is not implemented. (Dmitry)

Akhil P Oommen (6):
  dt-bindings: clk: qcom: Support gpu cx gdsc reset
  clk: qcom: Allow custom reset ops
  clk: qcom: gdsc: Add a reset op to poll gdsc collapse
  clk: qcom: gpucc-sc7280: Add cx collapse reset support
  dt-bindings: drm/msm/gpu: Add optional resets
  arm64: dts: qcom: sc7280: Add Reset support for gpu

 .../devicetree/bindings/display/msm/gpu.yaml   |  6 +
 arch/arm64/boot/dts/qcom/sc7280.dtsi   |  3 +++
 drivers/clk/qcom/gdsc.c| 23 ++
 drivers/clk/qcom/gdsc.h|  7 ++
 drivers/clk/qcom/gpucc-sc7280.c| 10 
 drivers/clk/qcom/reset.c   | 27 +-
 drivers/clk/qcom/reset.h   |  8 +++
 include/dt-bindings/clock/qcom,gpucc-sc7280.h  |  3 +++
 8 files changed, 82 insertions(+), 5 deletions(-)

-- 
2.7.4

Re: [Intel-gfx] [PATCH v2 2/2] drm/i915/slpc: Update the frequency debugfs