Re: [Intel-gfx] [PATCH 03/22] libiscsi: Make use of new the sg_map helper function
On 14/04/17 02:36 AM, Christoph Hellwig wrote: > On Thu, Apr 13, 2017 at 04:05:16PM -0600, Logan Gunthorpe wrote: >> Convert the kmap and kmap_atomic uses to the sg_map function. We now >> store the flags for the kmap instead of a boolean to indicate >> atomicitiy. We also propogate a possible kmap error down and create >> a new ISCSI_TCP_INTERNAL_ERR error type for this. > > Can you split out the new error handling into a separate prep patch > which should go to the iscsi maintainers ASAP? > Yes, I can do that. I'd just have thought they'd want to see the use case for the new error before accepting a patch like that... Logan ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 11/22] RDS: Make use of the new sg_map helper function
Straightforward conversion except there's no error path, so we WARN if
the sg_map fails.
Signed-off-by: Logan Gunthorpe
---
net/rds/ib_recv.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/net/rds/ib_recv.c b/net/rds/ib_recv.c
index e10624a..7f8fa99 100644
--- a/net/rds/ib_recv.c
+++ b/net/rds/ib_recv.c
@@ -801,9 +801,20 @@ static void rds_ib_cong_recv(struct rds_connection *conn,
to_copy = min(RDS_FRAG_SIZE - frag_off, PAGE_SIZE - map_off);
BUG_ON(to_copy & 7); /* Must be 64bit aligned. */
- addr = kmap_atomic(sg_page(&frag->f_sg));
+ addr = sg_map(&frag->f_sg, SG_KMAP_ATOMIC);
+ if (IS_ERR(addr)) {
+ /*
+* This should really never happen unless
+* the code is changed to use memory that is
+* not mappable in the sg. Seeing there doesn't
+* seem to be any error path out of here,
+* we can only WARN.
+*/
+ WARN(1, "Non-mappable memory used in sg!");
+ return;
+ }
- src = addr + frag->f_sg.offset + frag_off;
+ src = addr + frag_off;
dst = (void *)map->m_page_addrs[map_page] + map_off;
for (k = 0; k < to_copy; k += 8) {
/* Record ports that became uncongested, ie
@@ -811,7 +822,7 @@ static void rds_ib_cong_recv(struct rds_connection *conn,
uncongested |= ~(*src) & *dst;
*dst++ = *src++;
}
- kunmap_atomic(addr);
+ sg_unmap(&frag->f_sg, addr, SG_KMAP_ATOMIC);
copied += to_copy;
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/22] nvmet: Make use of the new sg_map helper function
On Thu, Apr 13, 2017 at 11:06:16PM -0600, Logan Gunthorpe wrote: > Or maybe I'll just send a patch for that > separately seeing it doesn't depend on anything and is pretty simple. I > can do that next week. Yes, please just send that patch linux-nvme, we should be able to get it into 4.12. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 04/22] target: Make use of the new sg_map function at 16 call sites
Fairly straightforward conversions in all spots. In a couple of cases
any error gets propogated up should sg_map fail. In other
cases a warning is issued if the kmap fails seeing there's no
clear error path. This should not be an issue until someone tries to
use unmappable memory in the sgl with this driver.
Signed-off-by: Logan Gunthorpe
---
drivers/target/iscsi/iscsi_target.c| 27 +---
drivers/target/target_core_rd.c| 3 +-
drivers/target/target_core_sbc.c | 122 +++--
drivers/target/target_core_transport.c | 18 +++--
drivers/target/target_core_user.c | 43
include/target/target_core_backend.h | 4 +-
6 files changed, 149 insertions(+), 68 deletions(-)
diff --git a/drivers/target/iscsi/iscsi_target.c
b/drivers/target/iscsi/iscsi_target.c
index a918024..e3e0d8f 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -579,7 +579,7 @@ iscsit_xmit_nondatain_pdu(struct iscsi_conn *conn, struct
iscsi_cmd *cmd,
}
static int iscsit_map_iovec(struct iscsi_cmd *, struct kvec *, u32, u32);
-static void iscsit_unmap_iovec(struct iscsi_cmd *);
+static void iscsit_unmap_iovec(struct iscsi_cmd *, struct kvec *);
static u32 iscsit_do_crypto_hash_sg(struct ahash_request *, struct iscsi_cmd *,
u32, u32, u32, u8 *);
static int
@@ -646,7 +646,7 @@ iscsit_xmit_datain_pdu(struct iscsi_conn *conn, struct
iscsi_cmd *cmd,
ret = iscsit_fe_sendpage_sg(cmd, conn);
- iscsit_unmap_iovec(cmd);
+ iscsit_unmap_iovec(cmd, &cmd->iov_data[1]);
if (ret < 0) {
iscsit_tx_thread_wait_for_tcp(conn);
@@ -925,7 +925,10 @@ static int iscsit_map_iovec(
while (data_length) {
u32 cur_len = min_t(u32, data_length, sg->length - page_off);
- iov[i].iov_base = kmap(sg_page(sg)) + sg->offset + page_off;
+ iov[i].iov_base = sg_map_offset(sg, page_off, SG_KMAP);
+ if (IS_ERR(iov[i].iov_base))
+ goto map_err;
+
iov[i].iov_len = cur_len;
data_length -= cur_len;
@@ -937,17 +940,25 @@ static int iscsit_map_iovec(
cmd->kmapped_nents = i;
return i;
+
+map_err:
+ cmd->kmapped_nents = i - 1;
+ iscsit_unmap_iovec(cmd, iov);
+ return -1;
}
-static void iscsit_unmap_iovec(struct iscsi_cmd *cmd)
+static void iscsit_unmap_iovec(struct iscsi_cmd *cmd, struct kvec *iov)
{
u32 i;
struct scatterlist *sg;
+ unsigned int page_off = cmd->first_data_sg_off;
sg = cmd->first_data_sg;
- for (i = 0; i < cmd->kmapped_nents; i++)
- kunmap(sg_page(&sg[i]));
+ for (i = 0; i < cmd->kmapped_nents; i++) {
+ sg_unmap_offset(&sg[i], iov[i].iov_base, page_off, SG_KMAP);
+ page_off = 0;
+ }
}
static void iscsit_ack_from_expstatsn(struct iscsi_conn *conn, u32 exp_statsn)
@@ -1610,7 +1621,7 @@ iscsit_get_dataout(struct iscsi_conn *conn, struct
iscsi_cmd *cmd,
rx_got = rx_data(conn, &cmd->iov_data[0], iov_count, rx_size);
- iscsit_unmap_iovec(cmd);
+ iscsit_unmap_iovec(cmd, iov);
if (rx_got != rx_size)
return -1;
@@ -2626,7 +2637,7 @@ static int iscsit_handle_immediate_data(
rx_got = rx_data(conn, &cmd->iov_data[0], iov_count, rx_size);
- iscsit_unmap_iovec(cmd);
+ iscsit_unmap_iovec(cmd, cmd->iov_data);
if (rx_got != rx_size) {
iscsit_rx_thread_wait_for_tcp(conn);
diff --git a/drivers/target/target_core_rd.c b/drivers/target/target_core_rd.c
index ddc216c..22c5ad5 100644
--- a/drivers/target/target_core_rd.c
+++ b/drivers/target/target_core_rd.c
@@ -431,7 +431,8 @@ static sense_reason_t rd_do_prot_rw(struct se_cmd *cmd,
bool is_read)
cmd->t_prot_sg, 0);
if (!rc)
- sbc_dif_copy_prot(cmd, sectors, is_read, prot_sg, prot_offset);
+ rc = sbc_dif_copy_prot(cmd, sectors, is_read, prot_sg,
+ prot_offset);
return rc;
}
diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index c194063..67cb420 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -420,17 +420,17 @@ static sense_reason_t xdreadwrite_callback(struct se_cmd
*cmd, bool success,
offset = 0;
for_each_sg(cmd->t_bidi_data_sg, sg, cmd->t_bidi_data_nents, count) {
- addr = kmap_atomic(sg_page(sg));
- if (!addr) {
+ addr = sg_map(sg, SG_KMAP_ATOMIC);
+ if (IS_ERR(addr)) {
ret = TCM_OUT_OF_RESOURCES;
goto out;
}
for (i = 0; i < sg->length; i++)
- *(addr + sg->offset + i) ^= *(buf + offset + i);
+ *(add
[Intel-gfx] [PATCH 09/22] dm-crypt: Make use of the new sg_map helper in 4 call sites
Very straightforward conversion to the new function in all four spots.
Signed-off-by: Logan Gunthorpe
---
drivers/md/dm-crypt.c | 38 +-
1 file changed, 25 insertions(+), 13 deletions(-)
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 389a363..6bd0ffc 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -589,9 +589,12 @@ static int crypt_iv_lmk_gen(struct crypt_config *cc, u8
*iv,
int r = 0;
if (bio_data_dir(dmreq->ctx->bio_in) == WRITE) {
- src = kmap_atomic(sg_page(&dmreq->sg_in));
- r = crypt_iv_lmk_one(cc, iv, dmreq, src + dmreq->sg_in.offset);
- kunmap_atomic(src);
+ src = sg_map(&dmreq->sg_in, SG_KMAP_ATOMIC);
+ if (IS_ERR(src))
+ return PTR_ERR(src);
+
+ r = crypt_iv_lmk_one(cc, iv, dmreq, src);
+ sg_unmap(&dmreq->sg_in, src, SG_KMAP_ATOMIC);
} else
memset(iv, 0, cc->iv_size);
@@ -607,14 +610,17 @@ static int crypt_iv_lmk_post(struct crypt_config *cc, u8
*iv,
if (bio_data_dir(dmreq->ctx->bio_in) == WRITE)
return 0;
- dst = kmap_atomic(sg_page(&dmreq->sg_out));
- r = crypt_iv_lmk_one(cc, iv, dmreq, dst + dmreq->sg_out.offset);
+ dst = sg_map(&dmreq->sg_out, SG_KMAP_ATOMIC);
+ if (IS_ERR(dst))
+ return PTR_ERR(dst);
+
+ r = crypt_iv_lmk_one(cc, iv, dmreq, dst);
/* Tweak the first block of plaintext sector */
if (!r)
- crypto_xor(dst + dmreq->sg_out.offset, iv, cc->iv_size);
+ crypto_xor(dst, iv, cc->iv_size);
- kunmap_atomic(dst);
+ sg_unmap(&dmreq->sg_out, dst, SG_KMAP_ATOMIC);
return r;
}
@@ -731,9 +737,12 @@ static int crypt_iv_tcw_gen(struct crypt_config *cc, u8
*iv,
/* Remove whitening from ciphertext */
if (bio_data_dir(dmreq->ctx->bio_in) != WRITE) {
- src = kmap_atomic(sg_page(&dmreq->sg_in));
- r = crypt_iv_tcw_whitening(cc, dmreq, src +
dmreq->sg_in.offset);
- kunmap_atomic(src);
+ src = sg_map(&dmreq->sg_in, SG_KMAP_ATOMIC);
+ if (IS_ERR(src))
+ return PTR_ERR(src);
+
+ r = crypt_iv_tcw_whitening(cc, dmreq, src);
+ sg_unmap(&dmreq->sg_in, src, SG_KMAP_ATOMIC);
}
/* Calculate IV */
@@ -755,9 +764,12 @@ static int crypt_iv_tcw_post(struct crypt_config *cc, u8
*iv,
return 0;
/* Apply whitening on ciphertext */
- dst = kmap_atomic(sg_page(&dmreq->sg_out));
- r = crypt_iv_tcw_whitening(cc, dmreq, dst + dmreq->sg_out.offset);
- kunmap_atomic(dst);
+ dst = sg_map(&dmreq->sg_out, SG_KMAP_ATOMIC);
+ if (IS_ERR(dst))
+ return PTR_ERR(dst);
+
+ r = crypt_iv_tcw_whitening(cc, dmreq, dst);
+ sg_unmap(&dmreq->sg_out, dst, SG_KMAP_ATOMIC);
return r;
}
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 10/22] staging: unisys: visorbus: Make use of the new sg_map helper function
Straightforward conversion to the new function.
Signed-off-by: Logan Gunthorpe
---
drivers/staging/unisys/visorhba/visorhba_main.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/unisys/visorhba/visorhba_main.c
b/drivers/staging/unisys/visorhba/visorhba_main.c
index 0ce92c8..2d8c8bc 100644
--- a/drivers/staging/unisys/visorhba/visorhba_main.c
+++ b/drivers/staging/unisys/visorhba/visorhba_main.c
@@ -842,7 +842,6 @@ do_scsi_nolinuxstat(struct uiscmdrsp *cmdrsp, struct
scsi_cmnd *scsicmd)
struct scatterlist *sg;
unsigned int i;
char *this_page;
- char *this_page_orig;
int bufind = 0;
struct visordisk_info *vdisk;
struct visorhba_devdata *devdata;
@@ -869,11 +868,14 @@ do_scsi_nolinuxstat(struct uiscmdrsp *cmdrsp, struct
scsi_cmnd *scsicmd)
sg = scsi_sglist(scsicmd);
for (i = 0; i < scsi_sg_count(scsicmd); i++) {
- this_page_orig = kmap_atomic(sg_page(sg + i));
- this_page = (void *)((unsigned long)this_page_orig |
-sg[i].offset);
+ this_page = sg_map(sg + i, SG_KMAP_ATOMIC);
+ if (IS_ERR(this_page)) {
+ scsicmd->result = DID_ERROR << 16;
+ return;
+ }
+
memcpy(this_page, buf + bufind, sg[i].length);
- kunmap_atomic(this_page_orig);
+ sg_unmap(sg + i, this_page, SG_KMAP_ATOMIC);
}
} else {
devdata = (struct visorhba_devdata *)scsidev->host->hostdata;
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/22] scatterlist: Introduce sg_map helper functions
On 14/04/17 02:35 AM, Christoph Hellwig wrote:
>> +
>> static inline int is_dma_buf_file(struct file *);
>>
>> struct dma_buf_list {
>
> I think the right fix here is to rename the operation to unmap_atomic
> and send out a little patch for that ASAP.
Ok, I can do that next week.
> I'd rather have separate functions for kmap vs kmap_atomic instead of
> the flags parameter. And while you're at it just always pass the 0
> offset parameter instead of adding a wrapper..
>
> Otherwise this looks good to me.
I settled on the flags because I thought the interface could be expanded
to do more things like automatically copy iomem to a bounce buffer (with
a flag). It'd also be possible to add things like vmap and
physical_address to the interface which would cover even more sg_page
users. All the implementations would then share the common offset
calculations, and switching between them becomes a matter of changing a
couple flags.
If you're still not convinced by the above arguments then I'll change
it but I did have reasons for choosing to do it this way.
I am fine with removing the offset versions. I will make that change.
Thanks,
Logan
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 17/22] mmc: sdhci: Make use of the new sg_map helper function
Straightforward conversion, except due to the lack of error path we
have to WARN if the memory in the SGL is not mappable.
Signed-off-by: Logan Gunthorpe
---
drivers/mmc/host/sdhci.c | 35 ++-
1 file changed, 30 insertions(+), 5 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 63bc33a..af0c107 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -497,15 +497,34 @@ static int sdhci_pre_dma_transfer(struct sdhci_host *host,
return sg_count;
}
+/*
+ * Note this function may return PTR_ERR and must be checked.
+ */
static char *sdhci_kmap_atomic(struct scatterlist *sg, unsigned long *flags)
{
+ void *ret;
+
local_irq_save(*flags);
- return kmap_atomic(sg_page(sg)) + sg->offset;
+
+ ret = sg_map(sg, SG_KMAP_ATOMIC);
+ if (IS_ERR(ret)) {
+ /*
+* This should really never happen unless the code is changed
+* to use memory that is not mappable in the sg. Seeing there
+* doesn't seem to be any error path out of here, we can only
+* WARN.
+*/
+ WARN(1, "Non-mappable memory used in sg!");
+ local_irq_restore(*flags);
+ }
+
+ return ret;
}
-static void sdhci_kunmap_atomic(void *buffer, unsigned long *flags)
+static void sdhci_kunmap_atomic(struct scatterlist *sg, void *buffer,
+ unsigned long *flags)
{
- kunmap_atomic(buffer);
+ sg_unmap(sg, buffer, SG_KMAP_ATOMIC);
local_irq_restore(*flags);
}
@@ -568,8 +587,11 @@ static void sdhci_adma_table_pre(struct sdhci_host *host,
if (offset) {
if (data->flags & MMC_DATA_WRITE) {
buffer = sdhci_kmap_atomic(sg, &flags);
+ if (IS_ERR(buffer))
+ return;
+
memcpy(align, buffer, offset);
- sdhci_kunmap_atomic(buffer, &flags);
+ sdhci_kunmap_atomic(sg, buffer, &flags);
}
/* tran, valid */
@@ -646,8 +668,11 @@ static void sdhci_adma_table_post(struct sdhci_host *host,
(sg_dma_address(sg) &
SDHCI_ADMA2_MASK);
buffer = sdhci_kmap_atomic(sg, &flags);
+ if (IS_ERR(buffer))
+ return;
+
memcpy(buffer, align, size);
- sdhci_kunmap_atomic(buffer, &flags);
+ sdhci_kunmap_atomic(sg, buffer, &flags);
align += SDHCI_ADMA2_ALIGN;
}
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 02/22] nvmet: Make use of the new sg_map helper function
This is a straight forward conversion in two places. Should kmap fail,
the code will return an INVALD_DATA error in the completion.
Signed-off-by: Logan Gunthorpe
---
drivers/nvme/target/fabrics-cmd.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/nvme/target/fabrics-cmd.c
b/drivers/nvme/target/fabrics-cmd.c
index 8bd022af..f62a634 100644
--- a/drivers/nvme/target/fabrics-cmd.c
+++ b/drivers/nvme/target/fabrics-cmd.c
@@ -122,7 +122,11 @@ static void nvmet_execute_admin_connect(struct nvmet_req
*req)
struct nvmet_ctrl *ctrl = NULL;
u16 status = 0;
- d = kmap(sg_page(req->sg)) + req->sg->offset;
+ d = sg_map(req->sg, SG_KMAP);
+ if (IS_ERR(d)) {
+ status = NVME_SC_SGL_INVALID_DATA;
+ goto out;
+ }
/* zero out initial completion result, assign values as needed */
req->rsp->result.u32 = 0;
@@ -158,7 +162,7 @@ static void nvmet_execute_admin_connect(struct nvmet_req
*req)
req->rsp->result.u16 = cpu_to_le16(ctrl->cntlid);
out:
- kunmap(sg_page(req->sg));
+ sg_unmap(req->sg, d, SG_KMAP);
nvmet_req_complete(req, status);
}
@@ -170,7 +174,11 @@ static void nvmet_execute_io_connect(struct nvmet_req *req)
u16 qid = le16_to_cpu(c->qid);
u16 status = 0;
- d = kmap(sg_page(req->sg)) + req->sg->offset;
+ d = sg_map(req->sg, SG_KMAP);
+ if (IS_ERR(d)) {
+ status = NVME_SC_SGL_INVALID_DATA;
+ goto out;
+ }
/* zero out initial completion result, assign values as needed */
req->rsp->result.u32 = 0;
@@ -205,7 +213,7 @@ static void nvmet_execute_io_connect(struct nvmet_req *req)
pr_info("adding queue %d to ctrl %d.\n", qid, ctrl->cntlid);
out:
- kunmap(sg_page(req->sg));
+ sg_unmap(req->sg, d, SG_KMAP);
nvmet_req_complete(req, status);
return;
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 14/22] scsi: arcmsr, ips, megaraid: Make use of the new sg_map helper function
Very straightforward conversion of three scsi drivers
Signed-off-by: Logan Gunthorpe
---
drivers/scsi/arcmsr/arcmsr_hba.c | 16
drivers/scsi/ips.c | 8
drivers/scsi/megaraid.c | 9 +++--
3 files changed, 23 insertions(+), 10 deletions(-)
diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
index af032c4..3cd485c 100644
--- a/drivers/scsi/arcmsr/arcmsr_hba.c
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -2306,7 +2306,10 @@ static int arcmsr_iop_message_xfer(struct
AdapterControlBlock *acb,
use_sg = scsi_sg_count(cmd);
sg = scsi_sglist(cmd);
- buffer = kmap_atomic(sg_page(sg)) + sg->offset;
+ buffer = sg_map(sg, SG_KMAP_ATOMIC);
+ if (IS_ERR(buffer))
+ return ARCMSR_MESSAGE_FAIL;
+
if (use_sg > 1) {
retvalue = ARCMSR_MESSAGE_FAIL;
goto message_out;
@@ -2539,7 +2542,7 @@ static int arcmsr_iop_message_xfer(struct
AdapterControlBlock *acb,
message_out:
if (use_sg) {
struct scatterlist *sg = scsi_sglist(cmd);
- kunmap_atomic(buffer - sg->offset);
+ sg_unmap(sg, buffer, SG_KMAP_ATOMIC);
}
return retvalue;
}
@@ -2590,11 +2593,16 @@ static void arcmsr_handle_virtual_command(struct
AdapterControlBlock *acb,
strncpy(&inqdata[32], "R001", 4); /* Product Revision */
sg = scsi_sglist(cmd);
- buffer = kmap_atomic(sg_page(sg)) + sg->offset;
+ buffer = sg_map(sg, SG_KMAP_ATOMIC);
+ if (IS_ERR(buffer)) {
+ cmd->result = (DID_ERROR << 16);
+ cmd->scsi_done(cmd);
+ return;
+ }
memcpy(buffer, inqdata, sizeof(inqdata));
sg = scsi_sglist(cmd);
- kunmap_atomic(buffer - sg->offset);
+ sg_unmap(sg, buffer, SG_KMAP_ATOMIC);
cmd->scsi_done(cmd);
}
diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
index 3419e1b..a44291d 100644
--- a/drivers/scsi/ips.c
+++ b/drivers/scsi/ips.c
@@ -1506,14 +1506,14 @@ static int ips_is_passthru(struct scsi_cmnd *SC)
/* kmap_atomic() ensures addressability of the user buffer.*/
/* local_irq_save() protects the KM_IRQ0 address slot. */
local_irq_save(flags);
-buffer = kmap_atomic(sg_page(sg)) + sg->offset;
-if (buffer && buffer[0] == 'C' && buffer[1] == 'O' &&
+buffer = sg_map(sg, SG_KMAP_ATOMIC);
+if (!IS_ERR(buffer) && buffer[0] == 'C' && buffer[1] == 'O' &&
buffer[2] == 'P' && buffer[3] == 'P') {
-kunmap_atomic(buffer - sg->offset);
+sg_unmap(sg, buffer, SG_KMAP_ATOMIC);
local_irq_restore(flags);
return 1;
}
-kunmap_atomic(buffer - sg->offset);
+sg_unmap(sg, buffer, SG_KMAP_ATOMIC);
local_irq_restore(flags);
}
return 0;
diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c
index 3c63c29..0b66e50 100644
--- a/drivers/scsi/megaraid.c
+++ b/drivers/scsi/megaraid.c
@@ -663,10 +663,15 @@ mega_build_cmd(adapter_t *adapter, Scsi_Cmnd *cmd, int
*busy)
struct scatterlist *sg;
sg = scsi_sglist(cmd);
- buf = kmap_atomic(sg_page(sg)) + sg->offset;
+ buf = sg_map(sg, SG_KMAP_ATOMIC);
+ if (IS_ERR(buf)) {
+cmd->result = (DID_ERROR << 16);
+ cmd->scsi_done(cmd);
+ return NULL;
+ }
memset(buf, 0, cmd->cmnd[4]);
- kunmap_atomic(buf - sg->offset);
+ sg_unmap(sg, buf, SG_KMAP_ATOMIC);
cmd->result = (DID_OK << 16);
cmd->scsi_done(cmd);
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 08/22] crypto: chcr: Make use of the new sg_map helper function
The get_page in this area looks *highly* suspect due to there being no
corresponding put_page. However, I've left that as is to avoid breaking
things.
I've also removed the KMAP_ATOMIC_ARGS check as it appears to be dead
code that dates back to when it was first committed...
Signed-off-by: Logan Gunthorpe
---
drivers/crypto/chelsio/chcr_algo.c | 28 +++-
1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/drivers/crypto/chelsio/chcr_algo.c
b/drivers/crypto/chelsio/chcr_algo.c
index 41bc7f4..a993d1d 100644
--- a/drivers/crypto/chelsio/chcr_algo.c
+++ b/drivers/crypto/chelsio/chcr_algo.c
@@ -1489,22 +1489,21 @@ static struct sk_buff *create_authenc_wr(struct
aead_request *req,
return ERR_PTR(-EINVAL);
}
-static void aes_gcm_empty_pld_pad(struct scatterlist *sg,
- unsigned short offset)
+static int aes_gcm_empty_pld_pad(struct scatterlist *sg,
+unsigned short offset)
{
- struct page *spage;
unsigned char *addr;
- spage = sg_page(sg);
- get_page(spage); /* so that it is not freed by NIC */
-#ifdef KMAP_ATOMIC_ARGS
- addr = kmap_atomic(spage, KM_SOFTIRQ0);
-#else
- addr = kmap_atomic(spage);
-#endif
- memset(addr + sg->offset, 0, offset + 1);
+ get_page(sg_page(sg)); /* so that it is not freed by NIC */
+
+ addr = sg_map(sg, SG_KMAP_ATOMIC);
+ if (IS_ERR(addr))
+ return PTR_ERR(addr);
+
+ memset(addr, 0, offset + 1);
+ sg_unmap(sg, addr, SG_KMAP_ATOMIC);
- kunmap_atomic(addr);
+ return 0;
}
static int set_msg_len(u8 *block, unsigned int msglen, int csize)
@@ -1940,7 +1939,10 @@ static struct sk_buff *create_gcm_wr(struct aead_request
*req,
if (req->cryptlen) {
write_sg_to_skb(skb, &frags, src, req->cryptlen);
} else {
- aes_gcm_empty_pld_pad(req->dst, authsize - 1);
+ err = aes_gcm_empty_pld_pad(req->dst, authsize - 1);
+ if (err)
+ goto dstmap_fail;
+
write_sg_to_skb(skb, &frags, reqctx->dst, crypt_len);
}
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/22] dm-crypt: Make use of the new sg_map helper in 4 call sites
On Thu, Apr 13, 2017 at 04:05:22PM -0600, Logan Gunthorpe wrote: > Very straightforward conversion to the new function in all four spots. I think the right fix here is to switch dm-crypt to the ahash API that takes a scatterlist. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 05/22] drm/i915: Make use of the new sg_map helper function
This is a single straightforward conversion from kmap to sg_map.
Signed-off-by: Logan Gunthorpe
---
drivers/gpu/drm/i915/i915_gem.c | 27 ---
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 67b1fc5..1b1b91a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2188,6 +2188,15 @@ static void __i915_gem_object_reset_page_iter(struct
drm_i915_gem_object *obj)
radix_tree_delete(&obj->mm.get_page.radix, iter.index);
}
+static void i915_gem_object_unmap(const struct drm_i915_gem_object *obj,
+ void *ptr)
+{
+ if (is_vmalloc_addr(ptr))
+ vunmap(ptr);
+ else
+ sg_unmap(obj->mm.pages->sgl, ptr, SG_KMAP);
+}
+
void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj,
enum i915_mm_subclass subclass)
{
@@ -2215,10 +2224,7 @@ void __i915_gem_object_put_pages(struct
drm_i915_gem_object *obj,
void *ptr;
ptr = ptr_mask_bits(obj->mm.mapping);
- if (is_vmalloc_addr(ptr))
- vunmap(ptr);
- else
- kunmap(kmap_to_page(ptr));
+ i915_gem_object_unmap(obj, ptr);
obj->mm.mapping = NULL;
}
@@ -2475,8 +2481,11 @@ static void *i915_gem_object_map(const struct
drm_i915_gem_object *obj,
void *addr;
/* A single page can always be kmapped */
- if (n_pages == 1 && type == I915_MAP_WB)
- return kmap(sg_page(sgt->sgl));
+ if (n_pages == 1 && type == I915_MAP_WB) {
+ addr = sg_map(sgt->sgl, SG_KMAP);
+ if (IS_ERR(addr))
+ return NULL;
+ }
if (n_pages > ARRAY_SIZE(stack_pages)) {
/* Too big for stack -- allocate temporary array instead */
@@ -2543,11 +2552,7 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object
*obj,
goto err_unpin;
}
- if (is_vmalloc_addr(ptr))
- vunmap(ptr);
- else
- kunmap(kmap_to_page(ptr));
-
+ i915_gem_object_unmap(obj, ptr);
ptr = obj->mm.mapping = NULL;
}
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 20/22] mmc: sdricoh_cs: Make use of the new sg_map helper function
This is a straightforward conversion to the new function.
Signed-off-by: Logan Gunthorpe
---
drivers/mmc/host/sdricoh_cs.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/mmc/host/sdricoh_cs.c b/drivers/mmc/host/sdricoh_cs.c
index 5ff26ab..7eeed23 100644
--- a/drivers/mmc/host/sdricoh_cs.c
+++ b/drivers/mmc/host/sdricoh_cs.c
@@ -319,16 +319,20 @@ static void sdricoh_request(struct mmc_host *mmc, struct
mmc_request *mrq)
for (i = 0; i < data->blocks; i++) {
size_t len = data->blksz;
u8 *buf;
- struct page *page;
int result;
- page = sg_page(data->sg);
- buf = kmap(page) + data->sg->offset + (len * i);
+ buf = sg_map_offset(data->sg, (len * i), SG_KMAP);
+ if (IS_ERR(buf)) {
+ cmd->error = PTR_ERR(buf);
+ break;
+ }
+
result =
sdricoh_blockio(host,
data->flags & MMC_DATA_READ, buf, len);
- kunmap(page);
- flush_dcache_page(page);
+ sg_unmap_offset(data->sg, buf, (len * i), SG_KMAP);
+
+ flush_dcache_page(sg_page(data->sg));
if (result) {
dev_err(dev, "sdricoh_request: cmd %i "
"block transfer failed\n", cmd->opcode);
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 19/22] mmc: tmio: Make use of the new sg_map helper function
Straightforward conversion to sg_map helper. A couple paths will
WARN if the memory does not end up being mappable.
Signed-off-by: Logan Gunthorpe
---
drivers/mmc/host/tmio_mmc.h | 12 ++--
drivers/mmc/host/tmio_mmc_dma.c | 5 +
drivers/mmc/host/tmio_mmc_pio.c | 24
3 files changed, 39 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 2b349d4..ba68c9fed 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -198,17 +198,25 @@ void tmio_mmc_enable_mmc_irqs(struct tmio_mmc_host *host,
u32 i);
void tmio_mmc_disable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
irqreturn_t tmio_mmc_irq(int irq, void *devid);
+/* Note: this function may return PTR_ERR and must be checked! */
static inline char *tmio_mmc_kmap_atomic(struct scatterlist *sg,
unsigned long *flags)
{
+ void *ret;
+
local_irq_save(*flags);
- return kmap_atomic(sg_page(sg)) + sg->offset;
+ ret = sg_map(sg, SG_KMAP_ATOMIC);
+
+ if (IS_ERR(ret))
+ local_irq_restore(*flags);
+
+ return ret;
}
static inline void tmio_mmc_kunmap_atomic(struct scatterlist *sg,
unsigned long *flags, void *virt)
{
- kunmap_atomic(virt - sg->offset);
+ sg_unmap(sg, virt, SG_KMAP_ATOMIC);
local_irq_restore(*flags);
}
diff --git a/drivers/mmc/host/tmio_mmc_dma.c b/drivers/mmc/host/tmio_mmc_dma.c
index fa8a936..07531f7 100644
--- a/drivers/mmc/host/tmio_mmc_dma.c
+++ b/drivers/mmc/host/tmio_mmc_dma.c
@@ -149,6 +149,11 @@ static void tmio_mmc_start_dma_tx(struct tmio_mmc_host
*host)
if (!aligned) {
unsigned long flags;
void *sg_vaddr = tmio_mmc_kmap_atomic(sg, &flags);
+ if (IS_ERR(sg_vaddr)) {
+ ret = PTR_ERR(sg_vaddr);
+ goto pio;
+ }
+
sg_init_one(&host->bounce_sg, host->bounce_buf, sg->length);
memcpy(host->bounce_buf, sg_vaddr, host->bounce_sg.length);
tmio_mmc_kunmap_atomic(sg, &flags, sg_vaddr);
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 6b789a7..d6fdbf6 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -479,6 +479,18 @@ static void tmio_mmc_pio_irq(struct tmio_mmc_host *host)
}
sg_virt = tmio_mmc_kmap_atomic(host->sg_ptr, &flags);
+ if (IS_ERR(sg_virt)) {
+ /*
+* This should really never happen unless
+* the code is changed to use memory that is
+* not mappable in the sg. Seeing there doesn't
+* seem to be any error path out of here,
+* we can only WARN.
+*/
+ WARN(1, "Non-mappable memory used in sg!");
+ return;
+ }
+
buf = (unsigned short *)(sg_virt + host->sg_off);
count = host->sg_ptr->length - host->sg_off;
@@ -506,6 +518,18 @@ static void tmio_mmc_check_bounce_buffer(struct
tmio_mmc_host *host)
if (host->sg_ptr == &host->bounce_sg) {
unsigned long flags;
void *sg_vaddr = tmio_mmc_kmap_atomic(host->sg_orig, &flags);
+ if (IS_ERR(sg_vaddr)) {
+ /*
+* This should really never happen unless
+* the code is changed to use memory that is
+* not mappable in the sg. Seeing there doesn't
+* seem to be any error path out of here,
+* we can only WARN.
+*/
+ WARN(1, "Non-mappable memory used in sg!");
+ return;
+ }
+
memcpy(sg_vaddr, host->bounce_buf, host->bounce_sg.length);
tmio_mmc_kunmap_atomic(host->sg_orig, &flags, sg_vaddr);
}
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/22] dm-crypt: Make use of the new sg_map helper in 4 call sites
On 14/04/17 02:39 AM, Christoph Hellwig wrote: > On Thu, Apr 13, 2017 at 04:05:22PM -0600, Logan Gunthorpe wrote: >> Very straightforward conversion to the new function in all four spots. > > I think the right fix here is to switch dm-crypt to the ahash API > that takes a scatterlist. Hmm, well I'm not sure I understand the code enough to make that conversion. But I was looking at it. One tricky bit seems to be that crypt_iv_lmk_one adds a seed, skips the first 16 bytes in the page and then hashes another 16 bytes from other data. What would you do construct a new sgl for it and pass it to the ahash api? The other thing is crypt_iv_lmk_post also seems to modify the page after the hash with a crypto_xor so you'd still need at least one kmap in there. Logan ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 00/22] Introduce common scatterlist map function
Hi Everyone, As part of my effort to enable P2P DMA transactions with PCI cards, we've identified the need to be able to safely put IO memory into scatterlists (and eventually other spots). This probably involves a conversion from struct page to pfn_t but that migration is a ways off and those decisions are yet to be made. As an initial step in that direction, I've started cleaning up some of the scatterlist code by trying to carve out a better defined layer between it and it's users. The longer term goal would be to remove sg_page or replace it with something that can potentially fail. This patchset is the first step in that effort. I've introduced a common function to map scatterlist memory and converted all the common kmap(sg_page()) cases. This removes about 66 sg_page calls (of ~331). Seeing this is a fairly large cleanup set that touches a wide swath of the kernel I have limited the people I've sent this to. I'd suggest we look toward merging the first patch and then I can send the individual subsystem patches on to their respective maintainers and get them merged independantly. (This is to avoid the conflicts I created with my last cleanup set... Sorry) Though, I'm certainly open to other suggestions to get it merged. The patchset is based on v4.11-rc6 and can be found in the sg_map branch from this git tree: https://github.com/sbates130272/linux-p2pmem.git Thanks, Logan Logan Gunthorpe (22): scatterlist: Introduce sg_map helper functions nvmet: Make use of the new sg_map helper function libiscsi: Make use of new the sg_map helper function target: Make use of the new sg_map function at 16 call sites drm/i915: Make use of the new sg_map helper function crypto: hifn_795x: Make use of the new sg_map helper function crypto: shash, caam: Make use of the new sg_map helper function crypto: chcr: Make use of the new sg_map helper function dm-crypt: Make use of the new sg_map helper in 4 call sites staging: unisys: visorbus: Make use of the new sg_map helper function RDS: Make use of the new sg_map helper function scsi: ipr, pmcraid, isci: Make use of the new sg_map helper in 4 call sites scsi: hisi_sas, mvsas, gdth: Make use of the new sg_map helper function scsi: arcmsr, ips, megaraid: Make use of the new sg_map helper function scsi: libfc, csiostor: Change to sg_copy_buffer in two drivers xen-blkfront: Make use of the new sg_map helper function mmc: sdhci: Make use of the new sg_map helper function mmc: spi: Make use of the new sg_map helper function mmc: tmio: Make use of the new sg_map helper function mmc: sdricoh_cs: Make use of the new sg_map helper function mmc: tifm_sd: Make use of the new sg_map helper function memstick: Make use of the new sg_map helper function crypto/shash.c | 9 +- drivers/block/xen-blkfront.c| 33 +-- drivers/crypto/caam/caamalg.c | 8 +- drivers/crypto/chelsio/chcr_algo.c | 28 +++--- drivers/crypto/hifn_795x.c | 32 --- drivers/dma-buf/dma-buf.c | 3 + drivers/gpu/drm/i915/i915_gem.c | 27 +++--- drivers/md/dm-crypt.c | 38 +--- drivers/memstick/host/jmb38x_ms.c | 23 - drivers/memstick/host/tifm_ms.c | 22 - drivers/mmc/host/mmc_spi.c | 26 +++-- drivers/mmc/host/sdhci.c| 35 ++- drivers/mmc/host/sdricoh_cs.c | 14 ++- drivers/mmc/host/tifm_sd.c | 88 + drivers/mmc/host/tmio_mmc.h | 12 ++- drivers/mmc/host/tmio_mmc_dma.c | 5 + drivers/mmc/host/tmio_mmc_pio.c | 24 + drivers/nvme/target/fabrics-cmd.c | 16 +++- drivers/scsi/arcmsr/arcmsr_hba.c| 16 +++- drivers/scsi/csiostor/csio_scsi.c | 54 +-- drivers/scsi/cxgbi/libcxgbi.c | 5 + drivers/scsi/gdth.c | 9 +- drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 14 ++- drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 13 ++- drivers/scsi/ipr.c | 27 +++--- drivers/scsi/ips.c | 8 +- drivers/scsi/isci/request.c | 42 drivers/scsi/libfc/fc_libfc.c | 49 ++ drivers/scsi/libiscsi_tcp.c | 32 --- drivers/scsi/megaraid.c | 9 +- drivers/scsi/mvsas/mv_sas.c | 10 +- drivers/scsi/pmcraid.c | 19 ++-- drivers/staging/unisys/visorhba/visorhba_main.c | 12 ++- drivers/target/iscsi/iscsi_target.c | 27 -- drivers/target/target_core_rd.c | 3 +- drivers/target/target_core_sbc.c| 122 +--- d
[Intel-gfx] [PATCH 15/22] scsi: libfc, csiostor: Change to sg_copy_buffer in two drivers
These two drivers appear to duplicate the functionality of
sg_copy_buffer. So we clean them up to use the common code.
This helps us remove a couple of instances that would otherwise be
slightly tricky sg_map usages.
Signed-off-by: Logan Gunthorpe
---
drivers/scsi/csiostor/csio_scsi.c | 54 +++
drivers/scsi/libfc/fc_libfc.c | 49 ---
2 files changed, 14 insertions(+), 89 deletions(-)
diff --git a/drivers/scsi/csiostor/csio_scsi.c
b/drivers/scsi/csiostor/csio_scsi.c
index a1ff75f..bd9d062 100644
--- a/drivers/scsi/csiostor/csio_scsi.c
+++ b/drivers/scsi/csiostor/csio_scsi.c
@@ -1489,60 +1489,14 @@ static inline uint32_t
csio_scsi_copy_to_sgl(struct csio_hw *hw, struct csio_ioreq *req)
{
struct scsi_cmnd *scmnd = (struct scsi_cmnd *)csio_scsi_cmnd(req);
- struct scatterlist *sg;
- uint32_t bytes_left;
- uint32_t bytes_copy;
- uint32_t buf_off = 0;
- uint32_t start_off = 0;
- uint32_t sg_off = 0;
- void *sg_addr;
- void *buf_addr;
struct csio_dma_buf *dma_buf;
+ size_t copied;
- bytes_left = scsi_bufflen(scmnd);
- sg = scsi_sglist(scmnd);
dma_buf = (struct csio_dma_buf *)csio_list_next(&req->gen_list);
+ copied = sg_copy_from_buffer(scsi_sglist(scmnd), scsi_sg_count(scmnd),
+dma_buf->vaddr, scsi_bufflen(scmnd));
- /* Copy data from driver buffer to SGs of SCSI CMD */
- while (bytes_left > 0 && sg && dma_buf) {
- if (buf_off >= dma_buf->len) {
- buf_off = 0;
- dma_buf = (struct csio_dma_buf *)
- csio_list_next(dma_buf);
- continue;
- }
-
- if (start_off >= sg->length) {
- start_off -= sg->length;
- sg = sg_next(sg);
- continue;
- }
-
- buf_addr = dma_buf->vaddr + buf_off;
- sg_off = sg->offset + start_off;
- bytes_copy = min((dma_buf->len - buf_off),
- sg->length - start_off);
- bytes_copy = min((uint32_t)(PAGE_SIZE - (sg_off & ~PAGE_MASK)),
-bytes_copy);
-
- sg_addr = kmap_atomic(sg_page(sg) + (sg_off >> PAGE_SHIFT));
- if (!sg_addr) {
- csio_err(hw, "failed to kmap sg:%p of ioreq:%p\n",
- sg, req);
- break;
- }
-
- csio_dbg(hw, "copy_to_sgl:sg_addr %p sg_off %d buf %p len %d\n",
- sg_addr, sg_off, buf_addr, bytes_copy);
- memcpy(sg_addr + (sg_off & ~PAGE_MASK), buf_addr, bytes_copy);
- kunmap_atomic(sg_addr);
-
- start_off += bytes_copy;
- buf_off += bytes_copy;
- bytes_left -= bytes_copy;
- }
-
- if (bytes_left > 0)
+ if (copied != scsi_bufflen(scmnd))
return DID_ERROR;
else
return DID_OK;
diff --git a/drivers/scsi/libfc/fc_libfc.c b/drivers/scsi/libfc/fc_libfc.c
index d623d08..ce0805a 100644
--- a/drivers/scsi/libfc/fc_libfc.c
+++ b/drivers/scsi/libfc/fc_libfc.c
@@ -113,45 +113,16 @@ u32 fc_copy_buffer_to_sglist(void *buf, size_t len,
u32 *nents, size_t *offset,
u32 *crc)
{
- size_t remaining = len;
- u32 copy_len = 0;
-
- while (remaining > 0 && sg) {
- size_t off, sg_bytes;
- void *page_addr;
-
- if (*offset >= sg->length) {
- /*
-* Check for end and drop resources
-* from the last iteration.
-*/
- if (!(*nents))
- break;
- --(*nents);
- *offset -= sg->length;
- sg = sg_next(sg);
- continue;
- }
- sg_bytes = min(remaining, sg->length - *offset);
-
- /*
-* The scatterlist item may be bigger than PAGE_SIZE,
-* but we are limited to mapping PAGE_SIZE at a time.
-*/
- off = *offset + sg->offset;
- sg_bytes = min(sg_bytes,
- (size_t)(PAGE_SIZE - (off & ~PAGE_MASK)));
- page_addr = kmap_atomic(sg_page(sg) + (off >> PAGE_SHIFT));
- if (crc)
- *crc = crc32(*crc, buf, sg_bytes);
- memcpy((char *)page_addr + (off & ~PAGE_MASK), buf, sg_bytes);
- kunmap_atomic(page_addr);
- buf += sg_bytes;
- *offset += sg_bytes;
- remaining -= sg_bytes;
- copy_len
[Intel-gfx] [PATCH 07/22] crypto: shash, caam: Make use of the new sg_map helper function
Very straightforward conversion to the new function in two crypto
drivers.
Signed-off-by: Logan Gunthorpe
---
crypto/shash.c| 9 ++---
drivers/crypto/caam/caamalg.c | 8 +++-
2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/crypto/shash.c b/crypto/shash.c
index 5e31c8d..2b7de94 100644
--- a/crypto/shash.c
+++ b/crypto/shash.c
@@ -283,10 +283,13 @@ int shash_ahash_digest(struct ahash_request *req, struct
shash_desc *desc)
if (nbytes < min(sg->length, ((unsigned int)(PAGE_SIZE)) - offset)) {
void *data;
- data = kmap_atomic(sg_page(sg));
- err = crypto_shash_digest(desc, data + offset, nbytes,
+ data = sg_map(sg, SG_KMAP_ATOMIC);
+ if (IS_ERR(data))
+ return PTR_ERR(data);
+
+ err = crypto_shash_digest(desc, data, nbytes,
req->result);
- kunmap_atomic(data);
+ sg_unmap(sg, data, SG_KMAP_ATOMIC);
crypto_yield(desc->flags);
} else
err = crypto_shash_init(desc) ?:
diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 9bc80eb..76b97de 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -89,7 +89,6 @@ static void dbg_dump_sg(const char *level, const char
*prefix_str,
struct scatterlist *sg, size_t tlen, bool ascii)
{
struct scatterlist *it;
- void *it_page;
size_t len;
void *buf;
@@ -98,19 +97,18 @@ static void dbg_dump_sg(const char *level, const char
*prefix_str,
* make sure the scatterlist's page
* has a valid virtual memory mapping
*/
- it_page = kmap_atomic(sg_page(it));
- if (unlikely(!it_page)) {
+ buf = sg_map(it, SG_KMAP_ATOMIC);
+ if (IS_ERR(buf)) {
printk(KERN_ERR "dbg_dump_sg: kmap failed\n");
return;
}
- buf = it_page + it->offset;
len = min_t(size_t, tlen, it->length);
print_hex_dump(level, prefix_str, prefix_type, rowsize,
groupsize, buf, len, ascii);
tlen -= len;
- kunmap_atomic(it_page);
+ sg_unmap(it, buf, SG_KMAP_ATOMIC);
}
}
#endif
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 13/22] scsi: hisi_sas, mvsas, gdth: Make use of the new sg_map helper function
Very straightforward conversion of three scsi drivers.
Signed-off-by: Logan Gunthorpe
---
drivers/scsi/gdth.c| 9 +++--
drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 14 +-
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 13 +
drivers/scsi/mvsas/mv_sas.c| 10 +-
4 files changed, 30 insertions(+), 16 deletions(-)
diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index d020a13..82c9fba 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -2301,10 +2301,15 @@ static void gdth_copy_internal_data(gdth_ha_str *ha,
Scsi_Cmnd *scp,
return;
}
local_irq_save(flags);
-address = kmap_atomic(sg_page(sl)) + sl->offset;
+address = sg_map(sl, SG_KMAP_ATOMIC);
+if (IS_ERR(address)) {
+scp->result = DID_ERROR << 16;
+return;
+ }
+
memcpy(address, buffer, cpnow);
flush_dcache_page(sg_page(sl));
-kunmap_atomic(address);
+sg_unmap(sl, address, SG_KMAP_ATOMIC);
local_irq_restore(flags);
if (cpsum == cpcount)
break;
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
index 854fbea..30408f8 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
@@ -1377,18 +1377,22 @@ static int slot_complete_v1_hw(struct hisi_hba
*hisi_hba,
void *to;
struct scatterlist *sg_resp = &task->smp_task.smp_resp;
- ts->stat = SAM_STAT_GOOD;
- to = kmap_atomic(sg_page(sg_resp));
+ to = sg_map(sg_resp, SG_KMAP_ATOMIC);
+ if (IS_ERR(to)) {
+ dev_err(dev, "slot complete: error mapping memory");
+ ts->stat = SAS_SG_ERR;
+ break;
+ }
+ ts->stat = SAM_STAT_GOOD;
dma_unmap_sg(dev, &task->smp_task.smp_resp, 1,
DMA_FROM_DEVICE);
dma_unmap_sg(dev, &task->smp_task.smp_req, 1,
DMA_TO_DEVICE);
- memcpy(to + sg_resp->offset,
- slot->status_buffer +
+ memcpy(to, slot->status_buffer +
sizeof(struct hisi_sas_err_record),
sg_dma_len(sg_resp));
- kunmap_atomic(to);
+ sg_unmap(sg_resp, to, SG_KMAP_ATOMIC);
break;
}
case SAS_PROTOCOL_SATA:
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index 1b21445..0907947 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -1796,18 +1796,23 @@ slot_complete_v2_hw(struct hisi_hba *hisi_hba, struct
hisi_sas_slot *slot,
struct scatterlist *sg_resp = &task->smp_task.smp_resp;
void *to;
+ to = sg_map(sg_resp, SG_KMAP_ATOMIC);
+ if (IS_ERR(to)) {
+ dev_err(dev, "slot complete: error mapping memory");
+ ts->stat = SAS_SG_ERR;
+ break;
+ }
+
ts->stat = SAM_STAT_GOOD;
- to = kmap_atomic(sg_page(sg_resp));
dma_unmap_sg(dev, &task->smp_task.smp_resp, 1,
DMA_FROM_DEVICE);
dma_unmap_sg(dev, &task->smp_task.smp_req, 1,
DMA_TO_DEVICE);
- memcpy(to + sg_resp->offset,
- slot->status_buffer +
+ memcpy(to, slot->status_buffer +
sizeof(struct hisi_sas_err_record),
sg_dma_len(sg_resp));
- kunmap_atomic(to);
+ sg_unmap(sg_resp, to, SG_KMAP_ATOMIC);
break;
}
case SAS_PROTOCOL_SATA:
diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
index c7cc803..374d0e0 100644
--- a/drivers/scsi/mvsas/mv_sas.c
+++ b/drivers/scsi/mvsas/mv_sas.c
@@ -1798,11 +1798,11 @@ int mvs_slot_complete(struct mvs_info *mvi, u32
rx_desc, u32 flags)
case SAS_PROTOCOL_SMP: {
struct scatterlist *sg_resp = &task->smp_task.smp_resp;
tstat->stat = SAM_STAT_GOOD;
- to = kmap_atomic(sg_page(sg_resp));
- memcpy(to + sg_resp->offset,
- slot->response + sizeof(struct mvs_err_info),
- sg_dma_len(sg_resp));
- kunmap_atomic(to);
+ to = sg_map(sg_resp, SG_KMAP_ATOMIC);
+ memcpy(to,
+ slot->response + sizeof(struct mvs_err_info),
+ sg_dma_len(sg_resp));
+ sg_unmap(sg_resp, to, SG_KMAP_ATOMIC
Re: [Intel-gfx] [PATCH 04/22] target: Make use of the new sg_map function at 16 call sites (fwd)
Thanks Julia. I missed that and I'll fix it in my series.
Logan
On 14/04/17 09:19 AM, Julia Lawall wrote:
> It looks like &udev->cmdr_lock should be released at line 512 if it has
> not been released otherwise. The lock was taken at line 438.
>
> julia
>
> -- Forwarded message --
> Date: Fri, 14 Apr 2017 22:21:44 +0800
> From: kbuild test robot
> To: kbu...@01.org
> Cc: Julia Lawall
> Subject: Re: [PATCH 04/22] target: Make use of the new sg_map function at 16
> call sites
>
> Hi Logan,
>
> [auto build test WARNING on scsi/for-next]
> [also build test WARNING on v4.11-rc6]
> [cannot apply to next-20170413]
> [if your patch is applied to the wrong git tree, please drop us a note to
> help improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Logan-Gunthorpe/Introduce-common-scatterlist-map-function/20170414-142518
> base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
> :: branch date: 8 hours ago
> :: commit date: 8 hours ago
>
>>> drivers/target/target_core_user.c:512:2-8: preceding lock on line 438
>drivers/target/target_core_user.c:512:2-8: preceding lock on line 471
>
> git remote add linux-review https://github.com/0day-ci/linux
> git remote update linux-review
> git checkout 78082134e7afdc59d744eb8d2def5c588e89c378
> vim +512 drivers/target/target_core_user.c
>
> 7c9e7a6f Andy Grover 2014-10-01 432
> sizeof(struct tcmu_cmd_entry));
> 7c9e7a6f Andy Grover 2014-10-01 433 command_size = base_command_size
> 7c9e7a6f Andy Grover 2014-10-01 434 +
> round_up(scsi_command_size(se_cmd->t_task_cdb), TCMU_OP_ALIGN_SIZE);
> 7c9e7a6f Andy Grover 2014-10-01 435
> 7c9e7a6f Andy Grover 2014-10-01 436 WARN_ON(command_size &
> (TCMU_OP_ALIGN_SIZE-1));
> 7c9e7a6f Andy Grover 2014-10-01 437
> 7c9e7a6f Andy Grover 2014-10-01 @438 spin_lock_irq(&udev->cmdr_lock);
> 7c9e7a6f Andy Grover 2014-10-01 439
> 7c9e7a6f Andy Grover 2014-10-01 440 mb = udev->mb_addr;
> 7c9e7a6f Andy Grover 2014-10-01 441 cmd_head = mb->cmd_head %
> udev->cmdr_size; /* UAM */
> 26418649 Sheng Yang 2016-02-26 442 data_length =
> se_cmd->data_length;
> 26418649 Sheng Yang 2016-02-26 443 if (se_cmd->se_cmd_flags &
> SCF_BIDI) {
> 26418649 Sheng Yang 2016-02-26 444
> BUG_ON(!(se_cmd->t_bidi_data_sg && se_cmd->t_bidi_data_nents));
> 26418649 Sheng Yang 2016-02-26 445 data_length +=
> se_cmd->t_bidi_data_sg->length;
> 26418649 Sheng Yang 2016-02-26 446 }
> 554617b2 Andy Grover 2016-08-25 447 if ((command_size >
> (udev->cmdr_size / 2)) ||
> 554617b2 Andy Grover 2016-08-25 448 data_length >
> udev->data_size) {
> 554617b2 Andy Grover 2016-08-25 449 pr_warn("TCMU: Request
> of size %zu/%zu is too big for %u/%zu "
> 3d9b9555 Andy Grover 2016-08-25 450 "cmd ring/data
> area\n", command_size, data_length,
> 7c9e7a6f Andy Grover 2014-10-01 451
> udev->cmdr_size, udev->data_size);
> 554617b2 Andy Grover 2016-08-25 452
> spin_unlock_irq(&udev->cmdr_lock);
> 554617b2 Andy Grover 2016-08-25 453 return
> TCM_INVALID_CDB_FIELD;
> 554617b2 Andy Grover 2016-08-25 454 }
> 7c9e7a6f Andy Grover 2014-10-01 455
> 26418649 Sheng Yang 2016-02-26 456 while
> (!is_ring_space_avail(udev, command_size, data_length)) {
> 7c9e7a6f Andy Grover 2014-10-01 457 int ret;
> 7c9e7a6f Andy Grover 2014-10-01 458 DEFINE_WAIT(__wait);
> 7c9e7a6f Andy Grover 2014-10-01 459
> 7c9e7a6f Andy Grover 2014-10-01 460
> prepare_to_wait(&udev->wait_cmdr, &__wait, TASK_INTERRUPTIBLE);
> 7c9e7a6f Andy Grover 2014-10-01 461
> 7c9e7a6f Andy Grover 2014-10-01 462 pr_debug("sleeping for
> ring space\n");
> 7c9e7a6f Andy Grover 2014-10-01 463
> spin_unlock_irq(&udev->cmdr_lock);
> 7c9e7a6f Andy Grover 2014-10-01 464 ret =
> schedule_timeout(msecs_to_jiffies(TCMU_TIME_OUT));
> 7c9e7a6f Andy Grover 2014-10-01 465
> finish_wait(&udev->wait_cmdr, &__wait);
> 7c9e7a6f Andy Grover 2014-10-01 466 if (!ret) {
> 7c9e7a6f Andy Grover 2014-10-01 467 pr_warn("tcmu:
> command timed out\n");
> 02eb924f Andy Grover 2016-10-06 468 return
> TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
> 7c9e7a6f Andy Grover 2014-10-01 469 }
> 7c9e7a6f Andy Grover 2014-10-01 470
> 7c9e7a6f Andy Grover 2014-10-01 471
> spin_lock_irq(&udev->cmdr_lock);
> 7c9e7a6f Andy Grover 2014-10-01 472
> 7c9e7a6f Andy Grover 2014-10-01 473 /* We dropped
> cmdr_lock, cmd_head is stale */
> 7c9e7a6f Andy Grover 2014-10-01 474
Re: [Intel-gfx] [PATCH 02/22] nvmet: Make use of the new sg_map helper function
On 13/04/17 10:59 PM, Christoph Hellwig wrote: > On Thu, Apr 13, 2017 at 04:05:15PM -0600, Logan Gunthorpe wrote: >> This is a straight forward conversion in two places. Should kmap fail, >> the code will return an INVALD_DATA error in the completion. > > It really should be using nvmet_copy_from_sgl to make things safer, > as we don't want to rely on any particular SG list layout. In fact > I'm pretty sure I did the conversion at some point, but it must never > have made it upstream. Ha, I did the conversion too a couple times for my RFC series. I can change this patch to do that. Or maybe I'll just send a patch for that separately seeing it doesn't depend on anything and is pretty simple. I can do that next week. Thanks, Logan ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 03/22] libiscsi: Make use of new the sg_map helper function
Convert the kmap and kmap_atomic uses to the sg_map function. We now
store the flags for the kmap instead of a boolean to indicate
atomicitiy. We also propogate a possible kmap error down and create
a new ISCSI_TCP_INTERNAL_ERR error type for this.
Signed-off-by: Logan Gunthorpe
---
drivers/scsi/cxgbi/libcxgbi.c | 5 +
drivers/scsi/libiscsi_tcp.c | 32
include/scsi/libiscsi_tcp.h | 3 ++-
3 files changed, 27 insertions(+), 13 deletions(-)
diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c
index bd7d39e..e38d0c1 100644
--- a/drivers/scsi/cxgbi/libcxgbi.c
+++ b/drivers/scsi/cxgbi/libcxgbi.c
@@ -1556,6 +1556,11 @@ static inline int read_pdu_skb(struct iscsi_conn *conn,
*/
iscsi_conn_printk(KERN_ERR, conn, "Invalid pdu or skb.");
return -EFAULT;
+ case ISCSI_TCP_INTERNAL_ERR:
+ pr_info("skb 0x%p, off %u, %d, TCP_INTERNAL_ERR.\n",
+ skb, offset, offloaded);
+ iscsi_conn_printk(KERN_ERR, conn, "Internal error.");
+ return -EFAULT;
case ISCSI_TCP_SEGMENT_DONE:
log_debug(1 << CXGBI_DBG_PDU_RX,
"skb 0x%p, off %u, %d, TCP_SEG_DONE, rc %d.\n",
diff --git a/drivers/scsi/libiscsi_tcp.c b/drivers/scsi/libiscsi_tcp.c
index 63a1d69..a2427699 100644
--- a/drivers/scsi/libiscsi_tcp.c
+++ b/drivers/scsi/libiscsi_tcp.c
@@ -133,25 +133,23 @@ static void iscsi_tcp_segment_map(struct iscsi_segment
*segment, int recv)
if (page_count(sg_page(sg)) >= 1 && !recv)
return;
- if (recv) {
- segment->atomic_mapped = true;
- segment->sg_mapped = kmap_atomic(sg_page(sg));
- } else {
- segment->atomic_mapped = false;
- /* the xmit path can sleep with the page mapped so use kmap */
- segment->sg_mapped = kmap(sg_page(sg));
+ /* the xmit path can sleep with the page mapped so don't use atomic */
+ segment->sg_map_flags = recv ? SG_KMAP_ATOMIC : SG_KMAP;
+ segment->sg_mapped = sg_map(sg, segment->sg_map_flags);
+
+ if (IS_ERR(segment->sg_mapped)) {
+ segment->sg_mapped = NULL;
+ return;
}
- segment->data = segment->sg_mapped + sg->offset + segment->sg_offset;
+ segment->data = segment->sg_mapped + segment->sg_offset;
}
void iscsi_tcp_segment_unmap(struct iscsi_segment *segment)
{
if (segment->sg_mapped) {
- if (segment->atomic_mapped)
- kunmap_atomic(segment->sg_mapped);
- else
- kunmap(sg_page(segment->sg));
+ sg_unmap(segment->sg, segment->sg_mapped,
+ segment->sg_map_flags);
segment->sg_mapped = NULL;
segment->data = NULL;
}
@@ -304,6 +302,9 @@ iscsi_tcp_segment_recv(struct iscsi_tcp_conn *tcp_conn,
break;
}
+ if (segment->data)
+ return -EFAULT;
+
copy = min(len - copied, segment->size - segment->copied);
ISCSI_DBG_TCP(tcp_conn->iscsi_conn, "copying %d\n", copy);
memcpy(segment->data + segment->copied, ptr + copied, copy);
@@ -927,6 +928,13 @@ int iscsi_tcp_recv_skb(struct iscsi_conn *conn, struct
sk_buff *skb,
avail);
rc = iscsi_tcp_segment_recv(tcp_conn, segment, ptr, avail);
BUG_ON(rc == 0);
+ if (rc < 0) {
+ ISCSI_DBG_TCP(conn, "memory fault. Consumed %d\n",
+ consumed);
+ *status = ISCSI_TCP_INTERNAL_ERR;
+ goto skb_done;
+ }
+
consumed += rc;
if (segment->total_copied >= segment->total_size) {
diff --git a/include/scsi/libiscsi_tcp.h b/include/scsi/libiscsi_tcp.h
index 30520d5..58c79af 100644
--- a/include/scsi/libiscsi_tcp.h
+++ b/include/scsi/libiscsi_tcp.h
@@ -47,7 +47,7 @@ struct iscsi_segment {
struct scatterlist *sg;
void*sg_mapped;
unsigned intsg_offset;
- boolatomic_mapped;
+ int sg_map_flags;
iscsi_segment_done_fn_t *done;
};
@@ -92,6 +92,7 @@ enum {
ISCSI_TCP_SKB_DONE, /* skb is out of data */
ISCSI_TCP_CONN_ERR, /* iscsi layer has fired a conn err */
ISCSI_TCP_SUSPENDED,/* conn is suspended */
+ ISCSI_TCP_INTERNAL_ERR, /* an internal error occurred */
};
extern void iscsi_tcp_hdr_recv_prep(struct iscsi_tcp_conn *tcp_conn);
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/22] nvmet: Make use of the new sg_map helper function
On Thu, Apr 13, 2017 at 04:05:15PM -0600, Logan Gunthorpe wrote: > This is a straight forward conversion in two places. Should kmap fail, > the code will return an INVALD_DATA error in the completion. It really should be using nvmet_copy_from_sgl to make things safer, as we don't want to rely on any particular SG list layout. In fact I'm pretty sure I did the conversion at some point, but it must never have made it upstream. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 12/22] scsi: ipr, pmcraid, isci: Make use of the new sg_map helper in 4 call sites
Very straightforward conversion of three scsi drivers.
Signed-off-by: Logan Gunthorpe
---
drivers/scsi/ipr.c | 27 ++-
drivers/scsi/isci/request.c | 42 +-
drivers/scsi/pmcraid.c | 19 ---
3 files changed, 51 insertions(+), 37 deletions(-)
diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
index b29afaf..f98f251 100644
--- a/drivers/scsi/ipr.c
+++ b/drivers/scsi/ipr.c
@@ -3853,7 +3853,7 @@ static void ipr_free_ucode_buffer(struct ipr_sglist
*sglist)
static int ipr_copy_ucode_buffer(struct ipr_sglist *sglist,
u8 *buffer, u32 len)
{
- int bsize_elem, i, result = 0;
+ int bsize_elem, i;
struct scatterlist *scatterlist;
void *kaddr;
@@ -3863,32 +3863,33 @@ static int ipr_copy_ucode_buffer(struct ipr_sglist
*sglist,
scatterlist = sglist->scatterlist;
for (i = 0; i < (len / bsize_elem); i++, buffer += bsize_elem) {
- struct page *page = sg_page(&scatterlist[i]);
+ kaddr = sg_map(&scatterlist[i], SG_KMAP);
+ if (IS_ERR(kaddr)) {
+ ipr_trace;
+ return PTR_ERR(kaddr);
+ }
- kaddr = kmap(page);
memcpy(kaddr, buffer, bsize_elem);
- kunmap(page);
+ sg_unmap(&scatterlist[i], kaddr, SG_KMAP);
scatterlist[i].length = bsize_elem;
-
- if (result != 0) {
- ipr_trace;
- return result;
- }
}
if (len % bsize_elem) {
- struct page *page = sg_page(&scatterlist[i]);
+ kaddr = sg_map(&scatterlist[i], SG_KMAP);
+ if (IS_ERR(kaddr)) {
+ ipr_trace;
+ return PTR_ERR(kaddr);
+ }
- kaddr = kmap(page);
memcpy(kaddr, buffer, len % bsize_elem);
- kunmap(page);
+ sg_unmap(&scatterlist[i], kaddr, SG_KMAP);
scatterlist[i].length = len % bsize_elem;
}
sglist->buffer_len = len;
- return result;
+ return 0;
}
/**
diff --git a/drivers/scsi/isci/request.c b/drivers/scsi/isci/request.c
index 47f66e9..66d6596 100644
--- a/drivers/scsi/isci/request.c
+++ b/drivers/scsi/isci/request.c
@@ -1424,12 +1424,14 @@ sci_stp_request_pio_data_in_copy_data_buffer(struct
isci_stp_request *stp_req,
sg = task->scatter;
while (total_len > 0) {
- struct page *page = sg_page(sg);
-
copy_len = min_t(int, total_len, sg_dma_len(sg));
- kaddr = kmap_atomic(page);
- memcpy(kaddr + sg->offset, src_addr, copy_len);
- kunmap_atomic(kaddr);
+ kaddr = sg_map(sg, SG_KMAP_ATOMIC);
+ if (IS_ERR(kaddr))
+ return SCI_FAILURE;
+
+ memcpy(kaddr, src_addr, copy_len);
+ sg_unmap(sg, kaddr, SG_KMAP_ATOMIC);
+
total_len -= copy_len;
src_addr += copy_len;
sg = sg_next(sg);
@@ -1771,14 +1773,16 @@ sci_io_request_frame_handler(struct isci_request *ireq,
case SCI_REQ_SMP_WAIT_RESP: {
struct sas_task *task = isci_request_access_task(ireq);
struct scatterlist *sg = &task->smp_task.smp_resp;
- void *frame_header, *kaddr;
+ void *frame_header;
u8 *rsp;
sci_unsolicited_frame_control_get_header(&ihost->uf_control,
frame_index,
&frame_header);
- kaddr = kmap_atomic(sg_page(sg));
- rsp = kaddr + sg->offset;
+ rsp = sg_map(sg, SG_KMAP_ATOMIC);
+ if (IS_ERR(rsp))
+ return SCI_FAILURE;
+
sci_swab32_cpy(rsp, frame_header, 1);
if (rsp[0] == SMP_RESPONSE) {
@@ -1814,7 +1818,7 @@ sci_io_request_frame_handler(struct isci_request *ireq,
ireq->sci_status =
SCI_FAILURE_CONTROLLER_SPECIFIC_IO_ERR;
sci_change_state(&ireq->sm, SCI_REQ_COMPLETED);
}
- kunmap_atomic(kaddr);
+ sg_unmap(sg, rsp, SG_KMAP_ATOMIC);
sci_controller_release_frame(ihost, frame_index);
@@ -2919,15 +2923,18 @@ static void isci_request_io_request_complete(struct
isci_host *ihost,
case SAS_PROTOCOL_SMP: {
struct scatterlist *sg = &task->smp_task.smp_req;
struct smp_req *smp_req;
- void *kaddr;
dma_unmap_sg(&ihost->pdev->dev, sg, 1, DMA_TO_DEVICE);
Re: [Intel-gfx] [PATCH 10/22] staging: unisys: visorbus: Make use of the new sg_map helper function
> -Original Message-
> From: Logan Gunthorpe [mailto:log...@deltatee.com]
...
> Subject: [PATCH 10/22] staging: unisys: visorbus: Make use of the new
> sg_map helper function
>
> Straightforward conversion to the new function.
>
> Signed-off-by: Logan Gunthorpe
Can you add Acked-by for this patch?
Acked-by: David Kershner
Tested on s-Par and no problems.
Thanks,
David Kershner
> ---
> drivers/staging/unisys/visorhba/visorhba_main.c | 12 +++-
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/unisys/visorhba/visorhba_main.c
> b/drivers/staging/unisys/visorhba/visorhba_main.c
> index 0ce92c8..2d8c8bc 100644
> --- a/drivers/staging/unisys/visorhba/visorhba_main.c
> +++ b/drivers/staging/unisys/visorhba/visorhba_main.c
> @@ -842,7 +842,6 @@ do_scsi_nolinuxstat(struct uiscmdrsp *cmdrsp, struct
> scsi_cmnd *scsicmd)
> struct scatterlist *sg;
> unsigned int i;
> char *this_page;
> - char *this_page_orig;
> int bufind = 0;
> struct visordisk_info *vdisk;
> struct visorhba_devdata *devdata;
> @@ -869,11 +868,14 @@ do_scsi_nolinuxstat(struct uiscmdrsp *cmdrsp,
> struct scsi_cmnd *scsicmd)
>
> sg = scsi_sglist(scsicmd);
> for (i = 0; i < scsi_sg_count(scsicmd); i++) {
> - this_page_orig = kmap_atomic(sg_page(sg + i));
> - this_page = (void *)((unsigned long)this_page_orig |
> - sg[i].offset);
> + this_page = sg_map(sg + i, SG_KMAP_ATOMIC);
> + if (IS_ERR(this_page)) {
> + scsicmd->result = DID_ERROR << 16;
> + return;
> + }
> +
> memcpy(this_page, buf + bufind, sg[i].length);
> - kunmap_atomic(this_page_orig);
> + sg_unmap(sg + i, this_page, SG_KMAP_ATOMIC);
> }
> } else {
> devdata = (struct visorhba_devdata *)scsidev->host-
> >hostdata;
> --
> 2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 18/22] mmc: spi: Make use of the new sg_map helper function
We use the sg_map helper but it's slightly more complicated
as we only check for the error when the mapping actually gets used.
Such that if the mapping failed but wasn't needed then no
error occurs.
Signed-off-by: Logan Gunthorpe
---
drivers/mmc/host/mmc_spi.c | 26 +++---
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
index e77d79c..82f786d 100644
--- a/drivers/mmc/host/mmc_spi.c
+++ b/drivers/mmc/host/mmc_spi.c
@@ -676,9 +676,15 @@ mmc_spi_writeblock(struct mmc_spi_host *host, struct
spi_transfer *t,
struct scratch *scratch = host->data;
u32 pattern;
- if (host->mmc->use_spi_crc)
+ if (host->mmc->use_spi_crc) {
+ if (IS_ERR(t->tx_buf))
+ return PTR_ERR(t->tx_buf);
+
scratch->crc_val = cpu_to_be16(
crc_itu_t(0, t->tx_buf, t->len));
+ t->tx_buf += t->len;
+ }
+
if (host->dma_dev)
dma_sync_single_for_device(host->dma_dev,
host->data_dma, sizeof(*scratch),
@@ -743,7 +749,6 @@ mmc_spi_writeblock(struct mmc_spi_host *host, struct
spi_transfer *t,
return status;
}
- t->tx_buf += t->len;
if (host->dma_dev)
t->tx_dma += t->len;
@@ -809,6 +814,11 @@ mmc_spi_readblock(struct mmc_spi_host *host, struct
spi_transfer *t,
}
leftover = status << 1;
+ if (bitshift || host->mmc->use_spi_crc) {
+ if (IS_ERR(t->rx_buf))
+ return PTR_ERR(t->rx_buf);
+ }
+
if (host->dma_dev) {
dma_sync_single_for_device(host->dma_dev,
host->data_dma, sizeof(*scratch),
@@ -860,9 +870,10 @@ mmc_spi_readblock(struct mmc_spi_host *host, struct
spi_transfer *t,
scratch->crc_val, crc, t->len);
return -EILSEQ;
}
+
+ t->rx_buf += t->len;
}
- t->rx_buf += t->len;
if (host->dma_dev)
t->rx_dma += t->len;
@@ -936,11 +947,11 @@ mmc_spi_data_do(struct mmc_spi_host *host, struct
mmc_command *cmd,
}
/* allow pio too; we don't allow highmem */
- kmap_addr = kmap(sg_page(sg));
+ kmap_addr = sg_map(sg, SG_KMAP);
if (direction == DMA_TO_DEVICE)
- t->tx_buf = kmap_addr + sg->offset;
+ t->tx_buf = kmap_addr;
else
- t->rx_buf = kmap_addr + sg->offset;
+ t->rx_buf = kmap_addr;
/* transfer each block, and update request status */
while (length) {
@@ -970,7 +981,8 @@ mmc_spi_data_do(struct mmc_spi_host *host, struct
mmc_command *cmd,
/* discard mappings */
if (direction == DMA_FROM_DEVICE)
flush_kernel_dcache_page(sg_page(sg));
- kunmap(sg_page(sg));
+ if (!IS_ERR(kmap_addr))
+ sg_unmap(sg, kmap_addr, SG_KMAP);
if (dma_dev)
dma_unmap_page(dma_dev, dma_addr, PAGE_SIZE, dir);
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/22] crypto: chcr: Make use of the new sg_map helper function
On Fri, Apr 14, 2017 at 3:35 AM, Logan Gunthorpe wrote:
> The get_page in this area looks *highly* suspect due to there being no
> corresponding put_page. However, I've left that as is to avoid breaking
> things.
chcr driver will post the request to LLD driver cxgb4 and put_page is
implemented there. it will no harm. Any how
we have removed the below code from driver.
http://www.mail-archive.com/linux-crypto@vger.kernel.org/msg24561.html
After this merge we can ignore your patch. Thanks
>
> I've also removed the KMAP_ATOMIC_ARGS check as it appears to be dead
> code that dates back to when it was first committed...
>
> Signed-off-by: Logan Gunthorpe
> ---
> drivers/crypto/chelsio/chcr_algo.c | 28 +++-
> 1 file changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/crypto/chelsio/chcr_algo.c
> b/drivers/crypto/chelsio/chcr_algo.c
> index 41bc7f4..a993d1d 100644
> --- a/drivers/crypto/chelsio/chcr_algo.c
> +++ b/drivers/crypto/chelsio/chcr_algo.c
> @@ -1489,22 +1489,21 @@ static struct sk_buff *create_authenc_wr(struct
> aead_request *req,
> return ERR_PTR(-EINVAL);
> }
>
> -static void aes_gcm_empty_pld_pad(struct scatterlist *sg,
> - unsigned short offset)
> +static int aes_gcm_empty_pld_pad(struct scatterlist *sg,
> +unsigned short offset)
> {
> - struct page *spage;
> unsigned char *addr;
>
> - spage = sg_page(sg);
> - get_page(spage); /* so that it is not freed by NIC */
> -#ifdef KMAP_ATOMIC_ARGS
> - addr = kmap_atomic(spage, KM_SOFTIRQ0);
> -#else
> - addr = kmap_atomic(spage);
> -#endif
> - memset(addr + sg->offset, 0, offset + 1);
> + get_page(sg_page(sg)); /* so that it is not freed by NIC */
> +
> + addr = sg_map(sg, SG_KMAP_ATOMIC);
> + if (IS_ERR(addr))
> + return PTR_ERR(addr);
> +
> + memset(addr, 0, offset + 1);
> + sg_unmap(sg, addr, SG_KMAP_ATOMIC);
>
> - kunmap_atomic(addr);
> + return 0;
> }
>
> static int set_msg_len(u8 *block, unsigned int msglen, int csize)
> @@ -1940,7 +1939,10 @@ static struct sk_buff *create_gcm_wr(struct
> aead_request *req,
> if (req->cryptlen) {
> write_sg_to_skb(skb, &frags, src, req->cryptlen);
> } else {
> - aes_gcm_empty_pld_pad(req->dst, authsize - 1);
> + err = aes_gcm_empty_pld_pad(req->dst, authsize - 1);
> + if (err)
> + goto dstmap_fail;
> +
> write_sg_to_skb(skb, &frags, reqctx->dst, crypt_len);
>
> }
> --
> 2.1.4
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 16/22] xen-blkfront: Make use of the new sg_map helper function
Straightforward conversion to the new helper, except due to
the lack of error path, we have to warn if unmapable memory
is ever present in the sgl.
Signed-off-by: Logan Gunthorpe
---
drivers/block/xen-blkfront.c | 33 +++--
1 file changed, 27 insertions(+), 6 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 5067a0a..7dcf41d 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -807,8 +807,19 @@ static int blkif_queue_rw_req(struct request *req, struct
blkfront_ring_info *ri
BUG_ON(sg->offset + sg->length > PAGE_SIZE);
if (setup.need_copy) {
- setup.bvec_off = sg->offset;
- setup.bvec_data = kmap_atomic(sg_page(sg));
+ setup.bvec_off = 0;
+ setup.bvec_data = sg_map(sg, SG_KMAP_ATOMIC);
+ if (IS_ERR(setup.bvec_data)) {
+ /*
+* This should really never happen unless
+* the code is changed to use memory that is
+* not mappable in the sg. Seeing there is a
+* questionable error path out of here,
+* we WARN.
+*/
+ WARN(1, "Non-mappable memory used in sg!");
+ return 1;
+ }
}
gnttab_foreach_grant_in_range(sg_page(sg),
@@ -818,7 +829,7 @@ static int blkif_queue_rw_req(struct request *req, struct
blkfront_ring_info *ri
&setup);
if (setup.need_copy)
- kunmap_atomic(setup.bvec_data);
+ sg_unmap(sg, setup.bvec_data, SG_KMAP_ATOMIC);
}
if (setup.segments)
kunmap_atomic(setup.segments);
@@ -1468,8 +1479,18 @@ static bool blkif_completion(unsigned long *id,
for_each_sg(s->sg, sg, num_sg, i) {
BUG_ON(sg->offset + sg->length > PAGE_SIZE);
- data.bvec_offset = sg->offset;
- data.bvec_data = kmap_atomic(sg_page(sg));
+ data.bvec_offset = 0;
+ data.bvec_data = sg_map(sg, SG_KMAP_ATOMIC);
+ if (IS_ERR(data.bvec_data)) {
+ /*
+* This should really never happen unless
+* the code is changed to use memory that is
+* not mappable in the sg. Seeing there is no
+* clear error path, we WARN.
+*/
+ WARN(1, "Non-mappable memory used in sg!");
+ return 1;
+ }
gnttab_foreach_grant_in_range(sg_page(sg),
sg->offset,
@@ -1477,7 +1498,7 @@ static bool blkif_completion(unsigned long *id,
blkif_copy_from_grant,
&data);
- kunmap_atomic(data.bvec_data);
+ sg_unmap(sg, data.bvec_data, SG_KMAP_ATOMIC);
}
}
/* Add the persistent grant into the list of free grants */
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/22] staging: unisys: visorbus: Make use of the new sg_map helper function
Great, thanks!
Logan
On 14/04/17 10:07 AM, Kershner, David A wrote:
> Can you add Acked-by for this patch?
>
> Acked-by: David Kershner
>
> Tested on s-Par and no problems.
>
> Thanks,
> David Kershner
>
>> ---
>> drivers/staging/unisys/visorhba/visorhba_main.c | 12 +++-
>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/staging/unisys/visorhba/visorhba_main.c
>> b/drivers/staging/unisys/visorhba/visorhba_main.c
>> index 0ce92c8..2d8c8bc 100644
>> --- a/drivers/staging/unisys/visorhba/visorhba_main.c
>> +++ b/drivers/staging/unisys/visorhba/visorhba_main.c
>> @@ -842,7 +842,6 @@ do_scsi_nolinuxstat(struct uiscmdrsp *cmdrsp, struct
>> scsi_cmnd *scsicmd)
>> struct scatterlist *sg;
>> unsigned int i;
>> char *this_page;
>> -char *this_page_orig;
>> int bufind = 0;
>> struct visordisk_info *vdisk;
>> struct visorhba_devdata *devdata;
>> @@ -869,11 +868,14 @@ do_scsi_nolinuxstat(struct uiscmdrsp *cmdrsp,
>> struct scsi_cmnd *scsicmd)
>>
>> sg = scsi_sglist(scsicmd);
>> for (i = 0; i < scsi_sg_count(scsicmd); i++) {
>> -this_page_orig = kmap_atomic(sg_page(sg + i));
>> -this_page = (void *)((unsigned long)this_page_orig |
>> - sg[i].offset);
>> +this_page = sg_map(sg + i, SG_KMAP_ATOMIC);
>> +if (IS_ERR(this_page)) {
>> +scsicmd->result = DID_ERROR << 16;
>> +return;
>> +}
>> +
>> memcpy(this_page, buf + bufind, sg[i].length);
>> -kunmap_atomic(this_page_orig);
>> +sg_unmap(sg + i, this_page, SG_KMAP_ATOMIC);
>> }
>> } else {
>> devdata = (struct visorhba_devdata *)scsidev->host-
>>> hostdata;
>> --
>> 2.1.4
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/22] libiscsi: Make use of new the sg_map helper function
On Thu, Apr 13, 2017 at 04:05:16PM -0600, Logan Gunthorpe wrote: > Convert the kmap and kmap_atomic uses to the sg_map function. We now > store the flags for the kmap instead of a boolean to indicate > atomicitiy. We also propogate a possible kmap error down and create > a new ISCSI_TCP_INTERNAL_ERR error type for this. Can you split out the new error handling into a separate prep patch which should go to the iscsi maintainers ASAP? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/22] dm-crypt: Make use of the new sg_map helper in 4 call sites
On 04/14/2017 06:03 PM, Logan Gunthorpe wrote: > > > On 14/04/17 02:39 AM, Christoph Hellwig wrote: >> On Thu, Apr 13, 2017 at 04:05:22PM -0600, Logan Gunthorpe wrote: >>> Very straightforward conversion to the new function in all four spots. >> >> I think the right fix here is to switch dm-crypt to the ahash API >> that takes a scatterlist. > > Hmm, well I'm not sure I understand the code enough to make that > conversion. But I was looking at it. One tricky bit seems to be that > crypt_iv_lmk_one adds a seed, skips the first 16 bytes in the page and > then hashes another 16 bytes from other data. What would you do > construct a new sgl for it and pass it to the ahash api? > > The other thing is crypt_iv_lmk_post also seems to modify the page after > the hash with a crypto_xor so you'd still need at least one kmap in there. yes, it is in fact modification of CBC mode implemented this hacky way. These IVs are only for compatibility with loopaes and very old trueCrypt formats. I think your patch is ok (if it is just plain conversion), if it is really needed, we can switch to ahash later in follow-up patch. All common code in dmcrypt uses async API already. p.s. there is a lot of lists on cc, but for this patch is missing dm-devel, dmcrypt changes need to go through Mike's tree (I added dm-devel to cc:) Milan ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/22] scatterlist: Introduce sg_map helper functions
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 0007b79..b95934b 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -37,6 +37,9 @@
>
> #include
>
> +/* Prevent the highmem.h macro from aliasing ops->kunmap_atomic */
> +#undef kunmap_atomic
> +
> static inline int is_dma_buf_file(struct file *);
>
> struct dma_buf_list {
I think the right fix here is to rename the operation to unmap_atomic
and send out a little patch for that ASAP.
> + * Flags can be any of:
> + * * SG_KMAP- Use kmap to create the mapping
> + * * SG_KMAP_ATOMIC - Use kmap_atomic to map the page atommically.
> + * Thus, the rules of that function apply: the cpu
> + * may not sleep until it is unmaped.
> + *
> + * Also, consider carefully whether this function is appropriate. It is
> + * largely not recommended for new code and if the sgl came from another
> + * subsystem and you don't know what kind of memory might be in the list
> + * then you definitely should not call it. Non-mappable memory may be in
> + * the sgl and thus this function may fail unexpectedly.
> + **/
> +static inline void *sg_map_offset(struct scatterlist *sg, size_t offset,
> +int flags)
I'd rather have separate functions for kmap vs kmap_atomic instead of
the flags parameter. And while you're at it just always pass the 0
offset parameter instead of adding a wrapper..
Otherwise this looks good to me.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/22] dm-crypt: Make use of the new sg_map helper in 4 call sites
Thanks for the information Milan. On 15/04/17 06:10 AM, Milan Broz wrote: > I think your patch is ok (if it is just plain conversion), if it is > really needed, we can switch to ahash later in follow-up patch. Sounds good to me. > p.s. > there is a lot of lists on cc, but for this patch is missing dm-devel, > dmcrypt changes > need to go through Mike's tree (I added dm-devel to cc:) Oh, sorry, I thought I had included all the lists. My hope however would be to get the first patch merged and then re-send the remaining patches to their respective maintainers. So that would have happened later. It's hard to manage patches otherwise with such large distribution lists. Logan ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 01/22] scatterlist: Introduce sg_map helper functions
This patch introduces functions which kmap the pages inside an sgl. Two
variants are provided: one if an offset is required and one if the
offset is zero. These functions replace a common pattern of
kmap(sg_page(sg)) that is used in about 50 places within the kernel.
The motivation for this work is to eventually safely support sgls that
contain io memory. In order for that to work, any access to the contents
of an iomem SGL will need to be done with iomemcpy or hit some warning.
(The exact details of how this will work have yet to be worked out.)
Having all the kmaps in one place is just a first step in that
direction. Additionally, seeing this helps cut down the users of sg_page,
it should make any effort to go to struct-page-less DMAs a little
easier (should that idea ever swing back into favour again).
A flags option is added to select between a regular or atomic mapping so
these functions can replace kmap(sg_page or kmap_atomic(sg_page.
Future work may expand this to have flags for using page_address or
vmap. Much further in the future, there may be a flag to allocate memory
and copy the data from/to iomem.
We also add the semantic that sg_map can fail to create a mapping,
despite the fact that the current code this is replacing is assumed to
never fail and the current version of these functions cannot fail. This
is to support iomem which either have to fail to create the mapping or
allocate memory as a bounce buffer which itself can fail.
Also, in terms of cleanup, a few of the existing kmap(sg_page) users
play things a bit loose in terms of whether they apply sg->offset
so using these helper functions should help avoid such issues.
Signed-off-by: Logan Gunthorpe
---
drivers/dma-buf/dma-buf.c | 3 ++
include/linux/scatterlist.h | 97 +
2 files changed, 100 insertions(+)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 0007b79..b95934b 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -37,6 +37,9 @@
#include
+/* Prevent the highmem.h macro from aliasing ops->kunmap_atomic */
+#undef kunmap_atomic
+
static inline int is_dma_buf_file(struct file *);
struct dma_buf_list {
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index cb3c8fe..acd4d73 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -5,6 +5,7 @@
#include
#include
#include
+#include
#include
struct scatterlist {
@@ -126,6 +127,102 @@ static inline struct page *sg_page(struct scatterlist *sg)
return (struct page *)((sg)->page_link & ~0x3);
}
+#define SG_KMAP(1 << 0)/* create a mapping with kmap */
+#define SG_KMAP_ATOMIC (1 << 1)/* create a mapping with kmap_atomic */
+
+/**
+ * sg_map_offset - kmap a page inside an sgl
+ * @sg:SG entry
+ * @offset:Offset into entry
+ * @flags: Flags for creating the mapping
+ *
+ * Description:
+ * Use this function to map a page in the scatterlist at the specified
+ * offset. sg->offset is already added for you. Note: the semantics of
+ * this function are that it may fail. Thus, its output should be checked
+ * with IS_ERR and PTR_ERR. Otherwise, a pointer to the specified offset
+ * in the mapped page is returned.
+ *
+ * Flags can be any of:
+ * * SG_KMAP- Use kmap to create the mapping
+ * * SG_KMAP_ATOMIC - Use kmap_atomic to map the page atommically.
+ *Thus, the rules of that function apply: the cpu
+ *may not sleep until it is unmaped.
+ *
+ * Also, consider carefully whether this function is appropriate. It is
+ * largely not recommended for new code and if the sgl came from another
+ * subsystem and you don't know what kind of memory might be in the list
+ * then you definitely should not call it. Non-mappable memory may be in
+ * the sgl and thus this function may fail unexpectedly.
+ **/
+static inline void *sg_map_offset(struct scatterlist *sg, size_t offset,
+ int flags)
+{
+ struct page *pg;
+ unsigned int pg_off;
+
+ offset += sg->offset;
+ pg = nth_page(sg_page(sg), offset >> PAGE_SHIFT);
+ pg_off = offset_in_page(offset);
+
+ if (flags & SG_KMAP_ATOMIC)
+ return kmap_atomic(pg) + pg_off;
+ else
+ return kmap(pg) + pg_off;
+}
+
+/**
+ * sg_unkmap_offset - unmap a page that was mapped with sg_map_offset
+ * @sg:SG entry
+ * @addr: address returned by sg_map_offset
+ * @offset:Offset into entry (same as specified for sg_map_offset)
+ * @flags: Flags, which are the same specified for sg_map_offset
+ *
+ * Description:
+ * Unmap the page that was mapped with sg_map_offset
+ *
+ **/
+static inline void sg_unmap_offset(struct scatterlist *sg, void *addr,
+ size_t offset, int flags)
+{
+ struct page *pg = nth_p
[Intel-gfx] [PATCH 22/22] memstick: Make use of the new sg_map helper function
Straightforward conversion, but we have to WARN if unmappable
memory finds its way into the sgl.
Signed-off-by: Logan Gunthorpe
---
drivers/memstick/host/jmb38x_ms.c | 23 ++-
drivers/memstick/host/tifm_ms.c | 22 +-
2 files changed, 35 insertions(+), 10 deletions(-)
diff --git a/drivers/memstick/host/jmb38x_ms.c
b/drivers/memstick/host/jmb38x_ms.c
index 48db922..256cf41 100644
--- a/drivers/memstick/host/jmb38x_ms.c
+++ b/drivers/memstick/host/jmb38x_ms.c
@@ -303,7 +303,6 @@ static int jmb38x_ms_transfer_data(struct jmb38x_ms_host
*host)
unsigned int off;
unsigned int t_size, p_cnt;
unsigned char *buf;
- struct page *pg;
unsigned long flags = 0;
if (host->req->long_data) {
@@ -318,14 +317,26 @@ static int jmb38x_ms_transfer_data(struct jmb38x_ms_host
*host)
unsigned int uninitialized_var(p_off);
if (host->req->long_data) {
- pg = nth_page(sg_page(&host->req->sg),
- off >> PAGE_SHIFT);
p_off = offset_in_page(off);
p_cnt = PAGE_SIZE - p_off;
p_cnt = min(p_cnt, length);
local_irq_save(flags);
- buf = kmap_atomic(pg) + p_off;
+ buf = sg_map_offset(&host->req->sg,
+off - host->req->sg.offset,
+SG_KMAP_ATOMIC);
+ if (IS_ERR(buf)) {
+ /*
+* This should really never happen unless
+* the code is changed to use memory that is
+* not mappable in the sg. Seeing there doesn't
+* seem to be any error path out of here,
+* we can only WARN.
+*/
+ WARN(1, "Non-mappable memory used in sg!");
+ break;
+ }
+
} else {
buf = host->req->data + host->block_pos;
p_cnt = host->req->data_len - host->block_pos;
@@ -341,7 +352,9 @@ static int jmb38x_ms_transfer_data(struct jmb38x_ms_host
*host)
: jmb38x_ms_read_reg_data(host, buf, p_cnt);
if (host->req->long_data) {
- kunmap_atomic(buf - p_off);
+ sg_unmap_offset(&host->req->sg, buf,
+off - host->req->sg.offset,
+SG_KMAP_ATOMIC);
local_irq_restore(flags);
}
diff --git a/drivers/memstick/host/tifm_ms.c b/drivers/memstick/host/tifm_ms.c
index 7bafa72..c0bc40e 100644
--- a/drivers/memstick/host/tifm_ms.c
+++ b/drivers/memstick/host/tifm_ms.c
@@ -186,7 +186,6 @@ static unsigned int tifm_ms_transfer_data(struct tifm_ms
*host)
unsigned int off;
unsigned int t_size, p_cnt;
unsigned char *buf;
- struct page *pg;
unsigned long flags = 0;
if (host->req->long_data) {
@@ -203,14 +202,25 @@ static unsigned int tifm_ms_transfer_data(struct tifm_ms
*host)
unsigned int uninitialized_var(p_off);
if (host->req->long_data) {
- pg = nth_page(sg_page(&host->req->sg),
- off >> PAGE_SHIFT);
p_off = offset_in_page(off);
p_cnt = PAGE_SIZE - p_off;
p_cnt = min(p_cnt, length);
local_irq_save(flags);
- buf = kmap_atomic(pg) + p_off;
+ buf = sg_map_offset(&host->req->sg,
+off - host->req->sg.offset,
+SG_KMAP_ATOMIC);
+ if (IS_ERR(buf)) {
+ /*
+* This should really never happen unless
+* the code is changed to use memory that is
+* not mappable in the sg. Seeing there doesn't
+* seem to be any error path out of here,
+* we can only WARN.
+*/
+ WARN(1, "Non-mappable memory used in sg!");
+ break;
+ }
} else {
buf = host->req->data + host->block_pos;
p_cnt = host->req->data_len - host->block_pos;
@@ -221,7 +231,9 @@ static unsigned int tifm_ms_transfer_data(struct tifm_ms
*host)
: tifm_ms_read_data(host, buf, p_cnt);
[Intel-gfx] [PATCH 21/22] mmc: tifm_sd: Make use of the new sg_map helper function
This conversion is a bit complicated. We modiy the read_fifo,
write_fifo and copy_page functions to take a scatterlist instead of a
page. Thus we can use sg_map instead of kmap_atomic. There's a bit of
accounting that needed to be done for the offset for this to work.
(Seeing sg_map takes care of the offset but it's already added and
used earlier in the code.
There's also no error path, so if unmappable memory finds its way into
the sgl we can only WARN.
Signed-off-by: Logan Gunthorpe
---
drivers/mmc/host/tifm_sd.c | 88 +++---
1 file changed, 67 insertions(+), 21 deletions(-)
diff --git a/drivers/mmc/host/tifm_sd.c b/drivers/mmc/host/tifm_sd.c
index 93c4b40..75b0d74 100644
--- a/drivers/mmc/host/tifm_sd.c
+++ b/drivers/mmc/host/tifm_sd.c
@@ -111,14 +111,26 @@ struct tifm_sd {
};
/* for some reason, host won't respond correctly to readw/writew */
-static void tifm_sd_read_fifo(struct tifm_sd *host, struct page *pg,
+static void tifm_sd_read_fifo(struct tifm_sd *host, struct scatterlist *sg,
unsigned int off, unsigned int cnt)
{
struct tifm_dev *sock = host->dev;
unsigned char *buf;
unsigned int pos = 0, val;
- buf = kmap_atomic(pg) + off;
+ buf = sg_map_offset(sg, off - sg->offset, SG_KMAP_ATOMIC);
+ if (IS_ERR(buf)) {
+ /*
+* This should really never happen unless
+* the code is changed to use memory that is
+* not mappable in the sg. Seeing there doesn't
+* seem to be any error path out of here,
+* we can only WARN.
+*/
+ WARN(1, "Non-mappable memory used in sg!");
+ return;
+ }
+
if (host->cmd_flags & DATA_CARRY) {
buf[pos++] = host->bounce_buf_data[0];
host->cmd_flags &= ~DATA_CARRY;
@@ -134,17 +146,29 @@ static void tifm_sd_read_fifo(struct tifm_sd *host,
struct page *pg,
}
buf[pos++] = (val >> 8) & 0xff;
}
- kunmap_atomic(buf - off);
+ sg_unmap_offset(sg, buf, off - sg->offset, SG_KMAP_ATOMIC);
}
-static void tifm_sd_write_fifo(struct tifm_sd *host, struct page *pg,
+static void tifm_sd_write_fifo(struct tifm_sd *host, struct scatterlist *sg,
unsigned int off, unsigned int cnt)
{
struct tifm_dev *sock = host->dev;
unsigned char *buf;
unsigned int pos = 0, val;
- buf = kmap_atomic(pg) + off;
+ buf = sg_map_offset(sg, off - sg->offset, SG_KMAP_ATOMIC);
+ if (IS_ERR(buf)) {
+ /*
+* This should really never happen unless
+* the code is changed to use memory that is
+* not mappable in the sg. Seeing there doesn't
+* seem to be any error path out of here,
+* we can only WARN.
+*/
+ WARN(1, "Non-mappable memory used in sg!");
+ return;
+ }
+
if (host->cmd_flags & DATA_CARRY) {
val = host->bounce_buf_data[0] | ((buf[pos++] << 8) & 0xff00);
writel(val, sock->addr + SOCK_MMCSD_DATA);
@@ -161,7 +185,7 @@ static void tifm_sd_write_fifo(struct tifm_sd *host, struct
page *pg,
val |= (buf[pos++] << 8) & 0xff00;
writel(val, sock->addr + SOCK_MMCSD_DATA);
}
- kunmap_atomic(buf - off);
+ sg_unmap_offset(sg, buf, off - sg->offset, SG_KMAP_ATOMIC);
}
static void tifm_sd_transfer_data(struct tifm_sd *host)
@@ -170,7 +194,6 @@ static void tifm_sd_transfer_data(struct tifm_sd *host)
struct scatterlist *sg = r_data->sg;
unsigned int off, cnt, t_size = TIFM_MMCSD_FIFO_SIZE * 2;
unsigned int p_off, p_cnt;
- struct page *pg;
if (host->sg_pos == host->sg_len)
return;
@@ -192,33 +215,57 @@ static void tifm_sd_transfer_data(struct tifm_sd *host)
}
off = sg[host->sg_pos].offset + host->block_pos;
- pg = nth_page(sg_page(&sg[host->sg_pos]), off >> PAGE_SHIFT);
p_off = offset_in_page(off);
p_cnt = PAGE_SIZE - p_off;
p_cnt = min(p_cnt, cnt);
p_cnt = min(p_cnt, t_size);
if (r_data->flags & MMC_DATA_READ)
- tifm_sd_read_fifo(host, pg, p_off, p_cnt);
+ tifm_sd_read_fifo(host, &sg[host->sg_pos], p_off,
+ p_cnt);
else if (r_data->flags & MMC_DATA_WRITE)
- tifm_sd_write_fifo(host, pg, p_off, p_cnt);
+ tifm_sd_write_fifo(host, &sg[host->sg_pos], p_off,
+ p_cnt);
t_size -= p_cnt;
host->block_pos += p_cnt;
}
}
-static void tifm_sd_copy_page(struct page *dst, unsigned
[Intel-gfx] [PATCH 06/22] crypto: hifn_795x: Make use of the new sg_map helper function
Conversion of a couple kmap_atomic instances to the sg_map helper
function.
However, it looks like there was a bug in the original code: the source
scatter lists offset (t->offset) was passed to ablkcipher_get which
added it to the destination address. This doesn't make a lot of
sense, but t->offset is likely always zero anyway. So, this patch cleans
that brokeness up.
Also, a change to the error path: if ablkcipher_get failed, everything
seemed to proceed as if it hadn't. Setting 'error' should hopefully
clear that up.
Signed-off-by: Logan Gunthorpe
---
drivers/crypto/hifn_795x.c | 32 +---
1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
index e09d405..8e2c6a9 100644
--- a/drivers/crypto/hifn_795x.c
+++ b/drivers/crypto/hifn_795x.c
@@ -1619,7 +1619,7 @@ static int hifn_start_device(struct hifn_device *dev)
return 0;
}
-static int ablkcipher_get(void *saddr, unsigned int *srestp, unsigned int
offset,
+static int ablkcipher_get(void *saddr, unsigned int *srestp,
struct scatterlist *dst, unsigned int size, unsigned int
*nbytesp)
{
unsigned int srest = *srestp, nbytes = *nbytesp, copy;
@@ -1632,15 +1632,17 @@ static int ablkcipher_get(void *saddr, unsigned int
*srestp, unsigned int offset
while (size) {
copy = min3(srest, dst->length, size);
- daddr = kmap_atomic(sg_page(dst));
- memcpy(daddr + dst->offset + offset, saddr, copy);
- kunmap_atomic(daddr);
+ daddr = sg_map(dst, SG_KMAP_ATOMIC);
+ if (IS_ERR(daddr))
+ return PTR_ERR(daddr);
+
+ memcpy(daddr, saddr, copy);
+ sg_unmap(dst, daddr, SG_KMAP_ATOMIC);
nbytes -= copy;
size -= copy;
srest -= copy;
saddr += copy;
- offset = 0;
pr_debug("%s: copy: %u, size: %u, srest: %u, nbytes: %u.\n",
__func__, copy, size, srest, nbytes);
@@ -1671,11 +1673,12 @@ static inline void hifn_complete_sa(struct hifn_device
*dev, int i)
static void hifn_process_ready(struct ablkcipher_request *req, int error)
{
+ int err;
struct hifn_request_context *rctx = ablkcipher_request_ctx(req);
if (rctx->walk.flags & ASYNC_FLAGS_MISALIGNED) {
unsigned int nbytes = req->nbytes;
- int idx = 0, err;
+ int idx = 0;
struct scatterlist *dst, *t;
void *saddr;
@@ -1695,17 +1698,24 @@ static void hifn_process_ready(struct
ablkcipher_request *req, int error)
continue;
}
- saddr = kmap_atomic(sg_page(t));
+ saddr = sg_map(t, SG_KMAP_ATOMIC);
+ if (IS_ERR(saddr)) {
+ if (!error)
+ error = PTR_ERR(saddr);
+ break;
+ }
+
+ err = ablkcipher_get(saddr, &t->length,
+dst, nbytes, &nbytes);
+ sg_unmap(t, saddr, SG_KMAP_ATOMIC);
- err = ablkcipher_get(saddr, &t->length, t->offset,
- dst, nbytes, &nbytes);
if (err < 0) {
- kunmap_atomic(saddr);
+ if (!error)
+ error = err;
break;
}
idx += err;
- kunmap_atomic(saddr);
}
hifn_cipher_walk_exit(&rctx->walk);
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: uninitialized value on error path (rev3)
On Mon, 17 Apr 2017, Dan Carpenter wrote: > On Fri, Apr 14, 2017 at 08:13:43PM -, Patchwork wrote: >> == Series Details == >> >> Series: drm/i915: uninitialized value on error path (rev3) >> URL : https://patchwork.freedesktop.org/series/23038/ >> State : success > > These patchwork emails are sort of useless. I wouldn't have sent the > patch if it couldn't compile which is basically all this is testing. > It's not exercising the failure path. Yeah, the CI is not smart enough to know that just by looking at the patch, so it'll test them all. ;) BR, Jani. -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for Introduce common scatterlist map function
== Series Details == Series: Introduce common scatterlist map function URL : https://patchwork.freedesktop.org/series/23149/ State : success == Summary == Series 23149v1 Introduce common scatterlist map function https://patchwork.freedesktop.org/api/1.0/series/23149/revisions/1/mbox/ Test gem_exec_suspend: Subgroup basic-s4-devices: pass -> DMESG-WARN (fi-kbl-7560u) fdo#100125 fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:436s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:429s fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:577s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:505s fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:487s fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:481s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:407s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:402s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:427s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:494s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:466s fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:458s fi-kbl-7560u total:278 pass:267 dwarn:1 dfail:0 fail:0 skip:10 time:566s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:449s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time:573s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:465s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:494s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:431s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:534s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time:401s a4b2d83360ec1673de58ff43d7b6fd5deaba1ce6 drm-tip: 2017y-04m-18d-07h-04m-20s UTC integration manifest 985aff6 memstick: Make use of the new sg_map helper function d99a0e8 mmc: tifm_sd: Make use of the new sg_map helper function 014e3a9 mmc: sdricoh_cs: Make use of the new sg_map helper function 1d7900f mmc: tmio: Make use of the new sg_map helper function e8be903 mmc: spi: Make use of the new sg_map helper function 03c5725 mmc: sdhci: Make use of the new sg_map helper function 3a03103 xen-blkfront: Make use of the new sg_map helper function b6beebc scsi: libfc, csiostor: Change to sg_copy_buffer in two drivers 6be9a2e scsi: arcmsr, ips, megaraid: Make use of the new sg_map helper function 19210d8b scsi: hisi_sas, mvsas, gdth: Make use of the new sg_map helper function 94ad37e scsi: ipr, pmcraid, isci: Make use of the new sg_map helper in 4 call sites 5f41bd7 RDS: Make use of the new sg_map helper function 9819e2c staging: unisys: visorbus: Make use of the new sg_map helper function ae35a8d dm-crypt: Make use of the new sg_map helper in 4 call sites 8c4ba50 crypto: chcr: Make use of the new sg_map helper function 7e8b219 crypto: shash, caam: Make use of the new sg_map helper function ecc0787 crypto: hifn_795x: Make use of the new sg_map helper function 216578b drm/i915: Make use of the new sg_map helper function f7dac4e target: Make use of the new sg_map function at 16 call sites f8d009f libiscsi: Make use of new the sg_map helper function f885488 nvmet: Make use of the new sg_map helper function 868502b scatterlist: Introduce sg_map helper functions == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4509/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4] ACPI / bus: Introduce a list of ids for "always present" devices
On Mon, 2017-04-10 at 17:49 +0200, Hans de Goede wrote:
> Several cherrytrail devices (all of which ship with windows 10) hide
> the
> lpss pwm controller in ACPI, typically the _STA method looks like
> this:
CherryTrail
PWM
LPSS
>
> Method (_STA, 0, NotSerialized) // _STA: Status
> {
> If (OSID == One)
> {
> Return (Zero)
> }
>
> Return (0x0F)
> }
>
> Where OSID is some dark magic seen in all cherrytrail ACPI tables
> making
> the machine behave differently depending on which OS it *thinks* it is
> booting, this gets set in a number of ways which we cannot control, on
> some newer machines it simple hardcoded to "One" aka win10.
>
> This causes the PWM controller to get hidden, which means Linux cannot
> control the backlight level on cht based tablets / laptops.
>
> Since loading the driver for this does no harm (the only in kernel
> user
> of it is the i915 driver, which will only use it when it needs it),
> this
> commit makes acpi_bus_get_status() always set status to
> ACPI_STA_DEFAULT
> for the 80862288 device, fixing the lack of backlight control.
> +#ifdef CONFIG_X86
> +/*
> + * Some ACPI devices are hidden (status == 0x0) in recent BIOS-es
> because
> + * some recent windows drivers bind to one device but poke at
> multiple
Windows
> + * devices at the same time, so the others get hidden.
> + * We work around this by always reporting ACPI_STA_DEFAULT for these
> + * devices. Note this MUST only be done for devices where this is
> safe.
> + *
> + * This forcing of devices to be present is limited to specific CPU
> (SoC)
> + * models both to avoid potentially causing trouble on other models
> and
> + * because some HIDs are re-used on different SoCs for completely
> + * different devices.
> + */
> +struct always_present_device_id {
> + struct acpi_device_id hid[2];
> + struct x86_cpu_id cpu_id[2];
> +};
> +
> +#define ENTRY(hid, cpu_model) {
> \
> + { { hid, }, {} },
> \
> + { { X86_VENDOR_INTEL, 6, cpu_model, X86_FEATURE_ANY, }, {} },
> \
Can we use separate macro for this. i.e. ICPU() ?
Perhaps at some point we might switch to set of generic ICPU()-like
macros.
Moreover, it seems you may change it to use only one existing structure
ICPU(model, hid)
> +}
> +
> +static const struct always_present_device_id
> always_present_device_ids[] = {
> + /*
> + * Cherrytrail pwm directly poked by GPU driver in win10,
> + * but Linux uses a separate pwm driver, harmless if not
> used.
> + */
> + ENTRY("80862288", INTEL_FAM6_ATOM_AIRMONT),
> +};
> +#endif
> +
> +void acpi_set_device_status(struct acpi_device *adev, u32 sta)
> +{
> + u32 *status = (u32 *)&adev->status;
> +#ifdef CONFIG_X86
> + int i;
> +
> + /* acpi_match_device_ids checks status, so start with default
> */
> + *status = ACPI_STA_DEFAULT;
> + for (i = 0; i < ARRAY_SIZE(always_present_device_ids); i++) {
> + if (acpi_match_device_ids(adev,
> + always_present_device_ids[i].hid) == 0 &&
> + x86_match_cpu(always_present_device_ids[i].cpu_id
> )) {
I don't like this. It looks a bit hackish. If we need more, than one hid
per CPU, we might just supply an array
ICPU(model, hids)
> + dev_info(&adev->dev, "Device [%s] is in
> always present list setting status [%08x]\n",
> + adev->pnp.bus_id, ACPI_STA_DEFAULT);
> + return;
> + }
> + }
> +#endif
--
Andy Shevchenko
Intel Finland Oy
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: uninitialized value on error path (rev3)
On Tue, Apr 18, 2017 at 10:58:44AM +0300, Jani Nikula wrote: > On Mon, 17 Apr 2017, Dan Carpenter wrote: > > On Fri, Apr 14, 2017 at 08:13:43PM -, Patchwork wrote: > >> == Series Details == > >> > >> Series: drm/i915: uninitialized value on error path (rev3) > >> URL : https://patchwork.freedesktop.org/series/23038/ > >> State : success > > > > These patchwork emails are sort of useless. I wouldn't have sent the > > patch if it couldn't compile which is basically all this is testing. > > It's not exercising the failure path. > > Yeah, the CI is not smart enough to know that just by looking at the > patch, so it'll test them all. ;) > But could it just send responses if something fails? Or it could be a much shorter email: CI test successful. Use X-mailer-blahblah to filter successful tests. We don't need to expose all the other internal information unless something fails. regards, dan carpenter ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Updated drm-intel-testing
Hi all, First slice of 4.13 features: new uabi: - extend error state dumping to include non-batch buffers requested by userspace (Chris), so that mesa gets more useful error state dumps - reapply the link status patch, for handlig dp link failures (Manasi). This needs updated -modesetting to work correctly. - Add new _WC cache domain, our assumption that wc can be subsumed by the existing cache domains didn't pan out (Chris) feature work: - first pile of conversion to atomic properties for connectors (Maarten) - refactor dp link rate handling code and related areas (Jani) - split engine info into class and runtime stuff (Oscar Mateo) - more robust wait_for_register code (Chris, Michal Wajdeczko) - fix rcu issues in the shrinker and simplify locking (Joonas) - guc/huc for glk (Anusha) - enable atomic modesetting for vlv/chv (Ville), plus final fixes for that Happy testing! Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: uninitialized value on error path (rev3)
On Tue, 18 Apr 2017, Dan Carpenter wrote: > On Tue, Apr 18, 2017 at 10:58:44AM +0300, Jani Nikula wrote: >> On Mon, 17 Apr 2017, Dan Carpenter wrote: >> > On Fri, Apr 14, 2017 at 08:13:43PM -, Patchwork wrote: >> >> == Series Details == >> >> >> >> Series: drm/i915: uninitialized value on error path (rev3) >> >> URL : https://patchwork.freedesktop.org/series/23038/ >> >> State : success >> > >> > These patchwork emails are sort of useless. I wouldn't have sent the >> > patch if it couldn't compile which is basically all this is testing. >> > It's not exercising the failure path. >> >> Yeah, the CI is not smart enough to know that just by looking at the >> patch, so it'll test them all. ;) >> > > But could it just send responses if something fails? Or it could be a > much shorter email: > > CI test successful. > > Use X-mailer-blahblah to filter successful tests. > > We don't need to expose all the other internal information unless > something fails. Thanks, I passed on the feedback. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] tests/feat_profile.json: legacy features list for piglit summary feature
On Thu, Apr 13, 2017 at 12:54:22PM +0300, Jari Tahvanainen wrote:
> Daniel has posted empty feat_profile.json as template to be used.
> This is my understanding about the features and what tests are covering those.
>
> Usage: piglit summary feature json-filename output-directory results-directory
>
> Signed-off-by: Jari Tahvanainen
> ---
>
> This is the opening for the discussion related to (legacy)
> feature-tests mapping. Please contribute on both feature list and
> also regexp to be used for collecting the test results for
> those. Target_rate defines the red/green for the output.
It's better than nothing, so ack from my side, pls push. Next up is going
to every platform and feature owner and actually making this correct. Can
you pls take care of this?
-Daniel
>
> tests/feat_profile.json | 111 +++-
> 1 file changed, 110 insertions(+), 1 deletion(-)
>
> diff --git a/tests/feat_profile.json b/tests/feat_profile.json
> index 251dfd9..0ccd2ea 100644
> --- a/tests/feat_profile.json
> +++ b/tests/feat_profile.json
> @@ -1,7 +1,116 @@
> {
> -"psr" : {
> +"Android Sync Framework" : {
> +"include_tests" : "gem_exec_fence|gem_exec_async|sw_sync|fence",
> +"exclude_tests" : "kms_busy@extended|fbc",
> +"target_rate" : 90
> +},
> +"atomic" : {
> +"include_tests" : "kms|testdisplay",
> +"exclude_tests" : "kms_busy@extended|fbc",
> +"target_rate" : 90
> +},
> +"core" : {
> +"include_tests" : "core",
> +"exclude_tests" : "",
> +"target_rate" : 90
> +},
> +"debugfs" : {
> +"include_tests" : "debugfs",
> +"exclude_tests" : "",
> +"target_rate" : 90
> +},
> +"drm" : {
> +"include_tests" : "drm",
> +"exclude_tests" : "",
> +"target_rate" : 90
> +},
> +"drv" : {
> +"include_tests" : "drv",
> +"exclude_tests" : "",
> +"target_rate" : 90
> +},
> +"gem" : {
> +"include_tests" : "gem",
> +"exclude_tests" : "",
> +"target_rate" : 90
> +},
> +"hang" : {
> +"comment" : "gpu hang recovery",
> + "include_tests" : "hang",
> +"exclude_tests" : "",
> +"target_rate" : 90
> +},
> +"kms" : {
> +"include_tests" : "kms",
> +"exclude_tests" : "",
> +"target_rate" : 90
> +},
> +"kms-cursor" : {
> +"comment" : "cursor support",
> +"include_tests" : "kms_cursor",
> +"exclude_tests" : "",
> +"target_rate" : 90
> +},
> +"kms-pipe_color" : {
> +"include_tests" : "kms_pipe_color",
> +"exclude_tests" : "",
> +"target_rate" : 90
> +},
> +"kms-flip" : {
> +"comment" : "vblank timestamps and async flips",
> +"include_tests" : "kms_flip",
> +"exclude_tests" : "",
> +"target_rate" : 90
> +},
> +"pm" : {
> +"include_tests" : "pm_|suspend|resume",
> +"exclude_tests" : "",
> +"target_rate" : 90
> +},
> +"pm-fbc" : {
> +"include_tests" : "fbc",
> +"exclude_tests" : "",
> +"target_rate" : 90
> +},
> +"pm-dpms" : {
> +"include_tests" : "dpms",
> +"exclude_tests" : "",
> +"target_rate" : 90
> +},
> +"pm-lpsp" : {
> +"include_tests" : "lpsp",
> +"exclude_tests" : "",
> +"target_rate" : 90
> +},
> +"pm-psr" : {
> "include_tests" : "psr",
> "exclude_tests" : "",
> "target_rate" : 90
> +},
> +"pm-rpm" : {
> +"include_tests" : "rpm",
> +"exclude_tests" : "",
> +"target_rate" : 90
> +},
> +"pm-rps" : {
> +"include_tests" : "rps",
> +"exclude_tests" : "",
> +"target_rate" : 90
> +},
> +"pm-sseu" : {
> +"comment" : "subslice ungating - sseu",
> +"include_tests" : "sseu",
> +"exclude_tests" : "",
> +"target_rate" : 90
> +},
> +"prime" : {
> +"include_tests" : "prime",
> +"exclude_tests" : "",
> +"target_rate" : 90
> +},
> +"sysfs" : {
> +"include_tests" : "sysfs",
> +"exclude_tests" : "",
> +"target_rate" : 90
> }
> }
> --
> 2.7.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [maintainer-tools PATCH] dim: Expand drm-misc branch explanations
On Mon, Apr 17, 2017 at 11:39:45AM +0100, Daniel Stone wrote: > Hi Sean, > > On 14 April 2017 at 16:47, Sean Paul wrote: > > On Mon, Apr 10, 2017 at 12:05:43PM -0400, Sean Paul wrote: > >> Add a bit more colour to the -misc branch explanations, and add a merge > >> timeline > >> similar to the chart used in drm-intel. > > > > Friendly review ping. > > I'm extremely deprived of coffee today, but I couldn't find anything > wrong with this. Thanks for doing the diagram! > > Reviewed-by: Daniel Stone Applied, thx. -Daniel > > Cheers, > Daniel > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH xf86-video-amdgpu] Adapt to PixmapDirtyUpdateRec::src being a DrawablePtr
From: Michel Dänzer
Signed-off-by: Michel Dänzer
---
Chris / Ilia / Ben, this should be manageable for the intel/nouveau
drivers, right?
src/amdgpu_drv.h | 26 ++
src/amdgpu_kms.c | 18 +-
src/drmmode_display.c | 8 ++--
3 files changed, 41 insertions(+), 11 deletions(-)
diff --git a/src/amdgpu_drv.h b/src/amdgpu_drv.h
index e5c44dc36..9e088e71a 100644
--- a/src/amdgpu_drv.h
+++ b/src/amdgpu_drv.h
@@ -170,6 +170,32 @@ typedef enum {
#define AMDGPU_PIXMAP_SHARING 1
#define amdgpu_is_gpu_screen(screen) (screen)->isGPU
#define amdgpu_is_gpu_scrn(scrn) (scrn)->is_gpu
+
+static inline ScreenPtr
+amdgpu_dirty_master(PixmapDirtyUpdatePtr dirty)
+{
+#ifdef HAS_DIRTYTRACKING_DRAWABLE_SRC
+ ScreenPtr screen = dirty->src->pScreen;
+#else
+ ScreenPtr screen = dirty->src->drawable.pScreen;
+#endif
+
+ if (screen->current_master)
+ return screen->current_master;
+
+ return screen;
+}
+
+static inline Bool
+amdgpu_dirty_src_equals(PixmapDirtyUpdatePtr dirty, PixmapPtr pixmap)
+{
+#ifdef HAS_DIRTYTRACKING_DRAWABLE_SRC
+ return dirty->src == &pixmap->drawable;
+#else
+ return dirty->src == pixmap;
+#endif
+}
+
#else
#define amdgpu_is_gpu_screen(screen) 0
#define amdgpu_is_gpu_scrn(scrn) 0
diff --git a/src/amdgpu_kms.c b/src/amdgpu_kms.c
index 0182cbe0a..29d3d076f 100644
--- a/src/amdgpu_kms.c
+++ b/src/amdgpu_kms.c
@@ -448,7 +448,7 @@ dirty_region(PixmapDirtyUpdatePtr dirty)
static void
redisplay_dirty(PixmapDirtyUpdatePtr dirty, RegionPtr region)
{
- ScrnInfoPtr scrn = xf86ScreenToScrn(dirty->src->drawable.pScreen);
+ ScrnInfoPtr scrn = xf86ScreenToScrn(dirty->slave_dst->drawable.pScreen);
if (RegionNil(region))
goto out;
@@ -481,12 +481,12 @@ amdgpu_prime_scanout_update_abort(xf86CrtcPtr crtc, void
*event_data)
void
amdgpu_sync_shared_pixmap(PixmapDirtyUpdatePtr dirty)
{
- ScreenPtr master_screen = dirty->src->master_pixmap->drawable.pScreen;
+ ScreenPtr master_screen = amdgpu_dirty_master(dirty);
PixmapDirtyUpdatePtr ent;
RegionPtr region;
xorg_list_for_each_entry(ent, &master_screen->pixmap_dirty_list, ent) {
- if (ent->slave_dst != dirty->src)
+ if (!amdgpu_dirty_src_equals(dirty, ent->slave_dst))
continue;
region = dirty_region(ent);
@@ -501,7 +501,7 @@ amdgpu_sync_shared_pixmap(PixmapDirtyUpdatePtr dirty)
static Bool
master_has_sync_shared_pixmap(ScrnInfoPtr scrn, PixmapDirtyUpdatePtr dirty)
{
- ScreenPtr master_screen = dirty->src->master_pixmap->drawable.pScreen;
+ ScreenPtr master_screen = amdgpu_dirty_master(dirty);
return master_screen->SyncSharedPixmap != NULL;
}
@@ -517,7 +517,7 @@ slave_has_sync_shared_pixmap(ScrnInfoPtr scrn,
PixmapDirtyUpdatePtr dirty)
static void
call_sync_shared_pixmap(PixmapDirtyUpdatePtr dirty)
{
- ScreenPtr master_screen = dirty->src->master_pixmap->drawable.pScreen;
+ ScreenPtr master_screen = amdgpu_dirty_master(dirty);
master_screen->SyncSharedPixmap(dirty);
}
@@ -527,7 +527,7 @@ call_sync_shared_pixmap(PixmapDirtyUpdatePtr dirty)
static Bool
master_has_sync_shared_pixmap(ScrnInfoPtr scrn, PixmapDirtyUpdatePtr dirty)
{
- ScrnInfoPtr master_scrn =
xf86ScreenToScrn(dirty->src->master_pixmap->drawable.pScreen);
+ ScrnInfoPtr master_scrn = xf86ScreenToScrn(amdgpu_dirty_master(dirty));
return master_scrn->driverName == scrn->driverName;
}
@@ -581,7 +581,7 @@ amdgpu_prime_scanout_do_update(xf86CrtcPtr crtc, unsigned
scanout_id)
Bool ret = FALSE;
xorg_list_for_each_entry(dirty, &screen->pixmap_dirty_list, ent) {
- if (dirty->src == scanoutpix && dirty->slave_dst ==
+ if (amdgpu_dirty_src_equals(dirty, scanoutpix) &&
dirty->slave_dst ==
drmmode_crtc->scanout[scanout_id ^
drmmode_crtc->tear_free].pixmap) {
RegionPtr region;
@@ -738,10 +738,10 @@ amdgpu_dirty_update(ScrnInfoPtr scrn)
PixmapDirtyUpdatePtr region_ent = ent;
if (master_has_sync_shared_pixmap(scrn, ent)) {
- ScreenPtr master_screen =
ent->src->master_pixmap->drawable.pScreen;
+ ScreenPtr master_screen =
amdgpu_dirty_master(ent);
xorg_list_for_each_entry(region_ent,
&master_screen->pixmap_dirty_list, ent) {
- if (region_ent->slave_dst == ent->src)
+ if (amdgpu_dirty_src_equals(ent,
region_ent->slave_dst))
break;
}
}
diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index 5e0c4133b..06103ed56 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -681,
[Intel-gfx] [PATCH xserver] Make PixmapDirtyUpdateRec::src a DrawablePtr
From: Michel Dänzer
This allows making the master screen's pixmap_dirty_list entries
explicitly reflect that we're now tracking the root window instead of
the screen pixmap, in order to allow Present page flipping on master
outputs while there are active slave outputs.
Define HAS_DIRTYTRACKING_DRAWABLE_SRC for drivers to check, but leave
HAS_DIRTYTRACKING_ROTATION defined as well to make things slightly
easier for drivers.
Signed-off-by: Michel Dänzer
---
The Start/StopFlippingPixmapTracking related changes are only compile
tested. The rest is smoke tested with the modesetting, amdgpu and radeon
drivers.
dix/pixmap.c | 24 +++-
hw/xfree86/drivers/modesetting/driver.c | 10 +-
hw/xfree86/drivers/modesetting/drmmode_display.c | 5 +++--
hw/xfree86/drivers/modesetting/drmmode_display.h | 2 +-
include/pixmap.h | 5 +++--
include/pixmapstr.h | 3 ++-
include/scrnintstr.h | 6 +++---
randr/randrstr.h | 2 +-
randr/rrcrtc.c | 20 +---
9 files changed, 38 insertions(+), 39 deletions(-)
diff --git a/dix/pixmap.c b/dix/pixmap.c
index b67a2e8a6..81ac5e2d8 100644
--- a/dix/pixmap.c
+++ b/dix/pixmap.c
@@ -181,12 +181,12 @@ PixmapDirtyDamageDestroy(DamagePtr damage, void *closure)
}
Bool
-PixmapStartDirtyTracking(PixmapPtr src,
+PixmapStartDirtyTracking(DrawablePtr src,
PixmapPtr slave_dst,
int x, int y, int dst_x, int dst_y,
Rotation rotation)
{
-ScreenPtr screen = src->drawable.pScreen;
+ScreenPtr screen = src->pScreen;
PixmapDirtyUpdatePtr dirty_update;
RegionPtr damageregion;
RegionRec dstregion;
@@ -204,8 +204,7 @@ PixmapStartDirtyTracking(PixmapPtr src,
dirty_update->dst_y = dst_y;
dirty_update->rotation = rotation;
dirty_update->damage = DamageCreate(NULL, PixmapDirtyDamageDestroy,
-DamageReportNone,
-TRUE, src->drawable.pScreen,
+DamageReportNone, TRUE, screen,
dirty_update);
if (rotation != RR_Rotate_0) {
@@ -241,16 +240,15 @@ PixmapStartDirtyTracking(PixmapPtr src,
RegionUnion(damageregion, damageregion, &dstregion);
RegionUninit(&dstregion);
-DamageRegister(screen->root ? &screen->root->drawable : &src->drawable,
- dirty_update->damage);
+DamageRegister(src, dirty_update->damage);
xorg_list_add(&dirty_update->ent, &screen->pixmap_dirty_list);
return TRUE;
}
Bool
-PixmapStopDirtyTracking(PixmapPtr src, PixmapPtr slave_dst)
+PixmapStopDirtyTracking(DrawablePtr src, PixmapPtr slave_dst)
{
-ScreenPtr screen = src->drawable.pScreen;
+ScreenPtr screen = src->pScreen;
PixmapDirtyUpdatePtr ent, safe;
xorg_list_for_each_entry_safe(ent, safe, &screen->pixmap_dirty_list, ent) {
@@ -269,8 +267,8 @@ PixmapDirtyCopyArea(PixmapPtr dst,
PixmapDirtyUpdatePtr dirty,
RegionPtr dirty_region)
{
-ScreenPtr pScreen = dirty->src->drawable.pScreen;
-DrawablePtr src = pScreen->root ? &pScreen->root->drawable :
&dirty->src->drawable;
+DrawablePtr src = dirty->src;
+ScreenPtr pScreen = src->pScreen;
int n;
BoxPtr b;
GCPtr pGC;
@@ -309,7 +307,7 @@ PixmapDirtyCompositeRotate(PixmapPtr dst_pixmap,
PixmapDirtyUpdatePtr dirty,
RegionPtr dirty_region)
{
-ScreenPtr pScreen = dirty->src->drawable.pScreen;
+ScreenPtr pScreen = dirty->src->pScreen;
PictFormatPtr format = PictureWindowFormat(pScreen->root);
PicturePtr src, dst;
XID include_inferiors = IncludeInferiors;
@@ -318,7 +316,7 @@ PixmapDirtyCompositeRotate(PixmapPtr dst_pixmap,
int error;
src = CreatePicture(None,
-&pScreen->root->drawable,
+dirty->src,
format,
CPSubwindowMode,
&include_inferiors, serverClient, &error);
@@ -367,7 +365,7 @@ PixmapDirtyCompositeRotate(PixmapPtr dst_pixmap,
*/
Bool PixmapSyncDirtyHelper(PixmapDirtyUpdatePtr dirty)
{
-ScreenPtr pScreen = dirty->src->drawable.pScreen;
+ScreenPtr pScreen = dirty->src->pScreen;
RegionPtr region = DamageRegion(dirty->damage);
PixmapPtr dst;
SourceValidateProcPtr SourceValidate;
diff --git a/hw/xfree86/drivers/modesetting/driver.c
b/hw/xfree86/drivers/modesetting/driver.c
index d7030e5c2..8ce51a502 100644
--- a/hw/xfree86/drivers/modesetting/driver.c
+++ b/hw/xfree86/drivers/modesetting/driver.c
@@ -1219,12 +1219,12 @@ msDisableSharedPixmapFlipping(RRCrtcPtr crtc)
}
static Bool
-msStartFlippingPixmapTrack
[Intel-gfx] [PATCH i-g-t v2 2/4] gem_create: Test huge object creation
From: Tvrtko Ursulin
Verify that we reject attempts to create object larger than
INT_MAX * PAGE_SIZE since i915 currently cannot support that.
Also removed the skip on simulation since I don't know why
would that be needed here.
v2: Removed basic tag and adjusted commit msg.
Signed-off-by: Tvrtko Ursulin
Cc: Chris Wilson
Cc: Daniel Vetter
---
tests/gem_create.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/tests/gem_create.c b/tests/gem_create.c
index de7b82094545..844ae8ce86cb 100644
--- a/tests/gem_create.c
+++ b/tests/gem_create.c
@@ -44,6 +44,7 @@
#include
#include
#include
+#include
#include
@@ -95,10 +96,13 @@ static void invalid_flag_test(int fd)
static void invalid_size_test(int fd)
{
- int handle;
+ uint32_t handle;
handle = __gem_create(fd, 0);
igt_assert(!handle);
+
+ handle = __gem_create(fd, INT_MAX * 4096UL + 1);
+ igt_assert(!handle);
}
/*
@@ -146,8 +150,6 @@ igt_main
{
int fd = -1;
- igt_skip_on_simulation();
-
igt_fixture {
fd = drm_open_driver(DRIVER_INTEL);
}
--
2.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: uninitialized value on error path (rev3)
On 18/04/17 12:14, Dan Carpenter wrote: On Tue, Apr 18, 2017 at 10:58:44AM +0300, Jani Nikula wrote: On Mon, 17 Apr 2017, Dan Carpenter wrote: On Fri, Apr 14, 2017 at 08:13:43PM -, Patchwork wrote: == Series Details == Series: drm/i915: uninitialized value on error path (rev3) URL : https://patchwork.freedesktop.org/series/23038/ State : success These patchwork emails are sort of useless. I wouldn't have sent the patch if it couldn't compile which is basically all this is testing. It's not exercising the failure path. Yeah, the CI is not smart enough to know that just by looking at the patch, so it'll test them all. ;) But could it just send responses if something fails? Or it could be a much shorter email: CI test successful. Use X-mailer-blahblah to filter successful tests. I guess that could be an interesting thing to add. But what prevents you from dropping these emails if you don't want them? Their subject always starts with "Fi.CI.BAT: success". We don't need to expose all the other internal information unless something fails. The purpose of this email is to say the patch series actually got tested, making it easier for the person pushing the series that it passed the basic requirements. The alternative would be to ask people to check on patchwork every time, which is cumbersome. One reason also to "expose the internals" is that machines come and go as we often, we only have one machine of old platforms. So, a patch checking a fix for gen4 with no actual gen4 HW testing it would be pointless. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v2 2/4] gem_create: Test huge object creation
On Tue, Apr 18, 2017 at 11:29:34AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> Verify that we reject attempts to create object larger than
> INT_MAX * PAGE_SIZE since i915 currently cannot support that.
>
> Also removed the skip on simulation since I don't know why
> would that be needed here.
>
> v2: Removed basic tag and adjusted commit msg.
>
> Signed-off-by: Tvrtko Ursulin
> Cc: Chris Wilson
> Cc: Daniel Vetter
> ---
> tests/gem_create.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/tests/gem_create.c b/tests/gem_create.c
> index de7b82094545..844ae8ce86cb 100644
> --- a/tests/gem_create.c
> +++ b/tests/gem_create.c
> @@ -44,6 +44,7 @@
> #include
> #include
> #include
> +#include
>
> #include
>
> @@ -95,10 +96,13 @@ static void invalid_flag_test(int fd)
>
> static void invalid_size_test(int fd)
> {
> - int handle;
> + uint32_t handle;
>
> handle = __gem_create(fd, 0);
> igt_assert(!handle);
> +
> + handle = __gem_create(fd, INT_MAX * 4096UL + 1);
> + igt_assert(!handle);
I'm still not going to agree this is the correct behaviour. That it no
longer triggers the warn will be sufficient for you and Daniel without
enshrining a known bug.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915: Fix GCC 4.4 build issue with __intel_wait_for_register_fw
From: Tvrtko Ursulin
Move the BUILD_BUG_ONs for busy-wait duration outside the
_wait_for_atomic macro as discussed on the mailing list.
v2: Simplify the macro by omitting the ret__ local. (Chris Wilson)
Signed-off-by: Tvrtko Ursulin
Suggested-by: Michal Wajdeczko
Fixes: 1d1a9774e404 ("drm/i915: Extend intel_wait_for_register_fw() with fast
timeout")
Cc: Michal Wajdeczko
Cc: Chris Wilson
---
drivers/gpu/drm/i915/intel_drv.h | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f78d7c5f3805..60ba15dc6d05 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -88,7 +88,6 @@
int cpu, ret, timeout = (US) * 1000; \
u64 base; \
_WAIT_FOR_ATOMIC_CHECK(ATOMIC); \
- BUILD_BUG_ON((US) > 5); \
if (!(ATOMIC)) { \
preempt_disable(); \
cpu = smp_processor_id(); \
@@ -130,8 +129,14 @@
ret__; \
})
-#define wait_for_atomic(COND, MS) _wait_for_atomic((COND), (MS) * 1000, 1)
-#define wait_for_atomic_us(COND, US) _wait_for_atomic((COND), (US), 1)
+#define wait_for_atomic_us(COND, US) \
+({ \
+ BUILD_BUG_ON(!__builtin_constant_p(US)); \
+ BUILD_BUG_ON((US) > 5); \
+ _wait_for_atomic((COND), (US), 1); \
+})
+
+#define wait_for_atomic(COND, MS) wait_for_atomic_us((COND), (MS) * 1000)
#define KHz(x) (1000 * (x))
#define MHz(x) KHz(1000 * (x))
--
2.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Fix GCC 4.4 build issue with __intel_wait_for_register_fw
On Tue, Apr 18, 2017 at 11:52:11AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> Move the BUILD_BUG_ONs for busy-wait duration outside the
> _wait_for_atomic macro as discussed on the mailing list.
>
> v2: Simplify the macro by omitting the ret__ local. (Chris Wilson)
>
> Signed-off-by: Tvrtko Ursulin
> Suggested-by: Michal Wajdeczko
> Fixes: 1d1a9774e404 ("drm/i915: Extend intel_wait_for_register_fw() with fast
> timeout")
> Cc: Michal Wajdeczko
> Cc: Chris Wilson
> ---
Reviewed-by: Michal Wajdeczko
-Michal
> drivers/gpu/drm/i915/intel_drv.h | 11 ---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index f78d7c5f3805..60ba15dc6d05 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -88,7 +88,6 @@
> int cpu, ret, timeout = (US) * 1000; \
> u64 base; \
> _WAIT_FOR_ATOMIC_CHECK(ATOMIC); \
> - BUILD_BUG_ON((US) > 5); \
> if (!(ATOMIC)) { \
> preempt_disable(); \
> cpu = smp_processor_id(); \
> @@ -130,8 +129,14 @@
> ret__; \
> })
>
> -#define wait_for_atomic(COND, MS)_wait_for_atomic((COND), (MS) * 1000, 1)
> -#define wait_for_atomic_us(COND, US) _wait_for_atomic((COND), (US), 1)
> +#define wait_for_atomic_us(COND, US) \
> +({ \
> + BUILD_BUG_ON(!__builtin_constant_p(US)); \
> + BUILD_BUG_ON((US) > 5); \
> + _wait_for_atomic((COND), (US), 1); \
> +})
> +
> +#define wait_for_atomic(COND, MS) wait_for_atomic_us((COND), (MS) * 1000)
>
> #define KHz(x) (1000 * (x))
> #define MHz(x) KHz(1000 * (x))
> --
> 2.9.3
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Fix GCC 4.4 build issue with __intel_wait_for_register_fw (rev2)
== Series Details == Series: drm/i915: Fix GCC 4.4 build issue with __intel_wait_for_register_fw (rev2) URL : https://patchwork.freedesktop.org/series/23020/ State : failure == Summary == Series 23020v2 drm/i915: Fix GCC 4.4 build issue with __intel_wait_for_register_fw https://patchwork.freedesktop.org/api/1.0/series/23020/revisions/2/mbox/ Test gem_exec_suspend: Subgroup basic-s4-devices: dmesg-warn -> PASS (fi-kbl-7560u) fdo#100125 Test pm_rpm: Subgroup basic-pci-d3-state: pass -> INCOMPLETE (fi-bsw-n3050) fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:430s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:423s fi-bsw-n3050 total:241 pass:206 dwarn:0 dfail:0 fail:0 skip:34 fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:511s fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:484s fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:480s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:409s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:411s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:423s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:486s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:468s fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:459s fi-kbl-7560u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:563s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:451s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time:572s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:463s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:499s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:432s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:531s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time:398s 4d374295ace9e57d83483341e2ad07cad5baf912 drm-tip: 2017y-04m-18d-10h-08m-05s UTC integration manifest cf1a97e drm/i915: Fix GCC 4.4 build issue with __intel_wait_for_register_fw == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4510/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH V5] drm/i915: Disable stolen memory when i915 runs on qemu
Hi, > [Zhang, Xiong Y] Thanks for your teach and propose. > For smbios, could you teach me which type and field could be used ? qemu adds a specific subsystem id to all virtual devices, so you can use that to figure you are running on qemu. One good candidate to check is the host bridge (easy to find due to fixed pci address), another one is the isa bridge aka lpc (igd already searches for that one for other reasons). In fact there already is a check for qemu in intel_detect_pch() ... cheers, Gerd ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v2] igt/gem_exec_nop/headless: Verify GT performance in headless mode
From: Tvrtko Ursulin
Check that no-op execution speed is the same in headless mode
and with the display active.
v2:
* Set graphics mode for the test to disable blanking. (Imre)
* Use igt stats framework as suggested by Chris.
Signed-off-by: Tvrtko Ursulin
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100572
Cc: Imre Deak
Cc: Chris Wilson
---
tests/gem_exec_nop.c | 71
1 file changed, 71 insertions(+)
diff --git a/tests/gem_exec_nop.c b/tests/gem_exec_nop.c
index 5d0d4957545e..1710bf4599fb 100644
--- a/tests/gem_exec_nop.c
+++ b/tests/gem_exec_nop.c
@@ -112,6 +112,74 @@ static void single(int fd, uint32_t handle,
ring_name, count, time*1e6 / count);
}
+static double
+stable_nop_on_ring(int fd, uint32_t handle, unsigned int engine,
+ int timeout, int reps)
+{
+ igt_stats_t s;
+ double n;
+
+ igt_assert(reps >= 5);
+
+ igt_stats_init_with_size(&s, reps);
+ s.is_float = true;
+
+ while (reps--) {
+ unsigned long count;
+ double time;
+
+ time = nop_on_ring(fd, handle, engine, timeout, &count);
+ igt_stats_push_float(&s, time / count);
+ }
+
+ n = igt_stats_get_median(&s);
+ igt_stats_fini(&s);
+
+ return n;
+}
+
+#define assert_within_epsilon(x, ref, tolerance) \
+igt_assert_f((x) <= (1.0 + tolerance) * ref && \
+ (x) >= (1.0 - tolerance) * ref, \
+ "'%s' != '%s' (%f not within %f%% tolerance of %f)\n",\
+ #x, #ref, x, tolerance * 100.0, ref)
+
+static void headless(int fd, uint32_t handle)
+{
+ const struct intel_execution_engine *e = &intel_execution_engines[0];
+ unsigned int nr_connected = 0;
+ drmModeConnector *connector;
+ drmModeRes *res;
+ double n[2];
+
+ res = drmModeGetResources(fd);
+ igt_assert(res);
+
+ /* require at least one connected connector for the test */
+ for (int i = 0; i < res->count_connectors; i++) {
+ connector = drmModeGetConnectorCurrent(fd, res->connectors[i]);
+ if (connector->connection == DRM_MODE_CONNECTED)
+ nr_connected++;
+ drmModeFreeConnector(connector);
+ }
+ igt_require(nr_connected > 0);
+
+ /* set graphics mode to prevent blanking */
+ kmstest_set_vt_graphics_mode();
+
+ /* benchmark nops */
+ n[0] = stable_nop_on_ring(fd, handle, e->exec_id | e->flags, 1, 5);
+
+ /* force all connectors off */
+ kmstest_unset_all_crtcs(fd, res);
+
+ /* benchmark nops again */
+ n[1] = stable_nop_on_ring(fd, handle, e->exec_id | e->flags, 1, 5);
+
+ /* check that the two execution speeds are roughly the same */
+ assert_within_epsilon(n[1], n[0], 0.1f);
+}
+
static bool ignore_engine(int fd, unsigned engine)
{
if (engine == 0)
@@ -494,6 +562,9 @@ igt_main
igt_subtest("context-sequential")
sequential(device, handle, FORKED | CONTEXT, 150);
+ igt_subtest("headless")
+ headless(device, handle);
+
igt_fixture {
igt_stop_hang_detector();
gem_close(device, handle);
--
2.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 1/3] ACPI / bus: Introduce a list of ids for "always present" devices
Several Cherry Trail devices (all of which ship with Windows 10) hide the
LPSS PWM controller in ACPI, typically the _STA method looks like this:
Method (_STA, 0, NotSerialized) // _STA: Status
{
If (OSID == One)
{
Return (Zero)
}
Return (0x0F)
}
Where OSID is some dark magic seen in all Cherry Trail ACPI tables making
the machine behave differently depending on which OS it *thinks* it is
booting, this gets set in a number of ways which we cannot control, on
some newer machines it simple hardcoded to "One" aka win10.
This causes the PWM controller to get hidden, which means Linux cannot
control the backlight level on cht based tablets / laptops.
Since loading the driver for this does no harm (the only in kernel user
of it is the i915 driver, which will only use it when it needs it), this
commit makes acpi_bus_get_status() always set status to ACPI_STA_DEFAULT
for the 80862288 device, fixing the lack of backlight control.
Signed-off-by: Hans de Goede
---
Changes in v2:
-Use pr_debug instead of ACPI_DEBUG_PRINT
Changes in v3:
-Un-inline acpi_set_device_status and do the always_present_device_ids
table check inside the un-inlined version of it
Changes in v4:
-Use dev_info instead of pr_debug
-Not only check for ACPI HID but also for CPU (SoC) model so as to not
for devices present on other models then for which the quirk is intended and
to avoid enabling unrelated ACPI devices which happen to use the same HID
Changes in v5:
-Only do the dev_info once per device (acpi_set_device_status gets called
multiple times per device during boot)
Changes in v6:
-Allow specifying more then one CPU-model for a single HID
-Not only match the HID but also the UID, like on Cherry Trail, on some Bay
Trail Windows 10 tablets we need to enable the PWM controller to get working
backlight even though _STA returns 0. The Bay Trail SoC has 2 PWM controllers
and we only need the first one. UID matching will allows adding an entry for
Bay Trail which only enables the first PWM controller
---
drivers/acpi/bus.c | 65 +
include/acpi/acpi_bus.h | 6 +
2 files changed, 66 insertions(+), 5 deletions(-)
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 34fbe02..eb30630 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -34,6 +34,8 @@
#include
#include
#ifdef CONFIG_X86
+#include
+#include
#include
#endif
#include
@@ -132,6 +134,69 @@ int acpi_bus_get_status(struct acpi_device *device)
}
EXPORT_SYMBOL(acpi_bus_get_status);
+#ifdef CONFIG_X86
+/*
+ * Some ACPI devices are hidden (status == 0x0) in recent BIOS-es because
+ * some recent Windows drivers bind to one device but poke at multiple
+ * devices at the same time, so the others get hidden.
+ * We work around this by always reporting ACPI_STA_DEFAULT for these
+ * devices. Note this MUST only be done for devices where this is safe.
+ *
+ * This forcing of devices to be present is limited to specific CPU (SoC)
+ * models both to avoid potentially causing trouble on other models and
+ * because some HIDs are re-used on different SoCs for completely
+ * different devices.
+ */
+struct always_present_device_id {
+ struct acpi_device_id hid[2];
+ struct x86_cpu_id cpu_ids[2];
+ const char *uid;
+};
+
+#define ICPU(model){ X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY, }
+
+#define ENTRY(hid, uid, cpu_models) { \
+ { { hid, }, {} }, \
+ { cpu_models, {} }, \
+ uid,\
+}
+
+static const struct always_present_device_id always_present_device_ids[] = {
+ /*
+* Cherry Trail PWM directly poked by GPU driver in win10,
+* but Linux uses a separate PWM driver, harmless if not used.
+*/
+ ENTRY("80862288", "1", ICPU(INTEL_FAM6_ATOM_AIRMONT)),
+};
+#endif
+
+void acpi_set_device_status(struct acpi_device *adev, u32 sta)
+{
+ u32 *status = (u32 *)&adev->status;
+#ifdef CONFIG_X86
+ u32 old_status = *status;
+ int i;
+
+ /* acpi_match_device_ids checks status, so start with default */
+ *status = ACPI_STA_DEFAULT;
+ for (i = 0; i < ARRAY_SIZE(always_present_device_ids); i++) {
+ if (acpi_match_device_ids(adev,
+ always_present_device_ids[i].hid) == 0 &&
+ adev->pnp.unique_id &&
+ strcmp(adev->pnp.unique_id,
+ always_present_device_ids[i].uid) == 0 &&
+ x86_match_cpu(always_present_device_ids[i].cpu_ids)) {
+ if (old_status != ACPI_STA_DEFAULT)
+ dev_info(&adev->dev,
+"Device [%s] is in always present list
setting status [%08x]\n",
+
[Intel-gfx] [PATCH v6 3/3] ACPI / bus: Add Bay Trail PWM controller to list of always-present devices
Just like on Cherry Trail, on some Bay Trail Windows 10 tablets we
need to enable the PWM controller to get working backlight even
though _STA returns 0.
Add an entry for the the Bay Trail PWM controller to list of
always-present devices to fix backlight control not working on
some Bay Trail devices.
Signed-off-by: Hans de Goede
---
Changes in v6:
-This is a new patch in v6 of this patch-set
---
drivers/acpi/bus.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 6fa177c..5f9fe3f 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -163,9 +163,10 @@ struct always_present_device_id {
static const struct always_present_device_id always_present_device_ids[] = {
/*
-* Cherry Trail PWM directly poked by GPU driver in win10,
+* Bay / Cherry Trail PWM directly poked by GPU driver in win10,
* but Linux uses a separate PWM driver, harmless if not used.
*/
+ ENTRY("80860F09", "1", ICPU(INTEL_FAM6_ATOM_SILVERMONT1)),
ENTRY("80862288", "1", ICPU(INTEL_FAM6_ATOM_AIRMONT)),
/*
* The INT0002 device is necessary to clear wakeup interrupt sources
--
2.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 2/3] ACPI / bus: Add INT0002 to list of always-present devices
The INT0002 device is necessary to clear wakeup interrupt sources
on Cherry Trail devices, without it we get nobody cared IRQ msgs
and some systems don't properly resume at all without it.
Signed-off-by: Hans de Goede
---
Changes in v6:
-This is a new patch in v6 of this patch-set
---
drivers/acpi/bus.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index eb30630..6fa177c 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -167,6 +167,11 @@ static const struct always_present_device_id
always_present_device_ids[] = {
* but Linux uses a separate PWM driver, harmless if not used.
*/
ENTRY("80862288", "1", ICPU(INTEL_FAM6_ATOM_AIRMONT)),
+ /*
+* The INT0002 device is necessary to clear wakeup interrupt sources
+* on Cherry Trail devices, without it we get nobody cared IRQ msgs.
+*/
+ ENTRY("INT0002", "1", ICPU(INTEL_FAM6_ATOM_AIRMONT)),
};
#endif
--
2.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Fix system hang with EI UP masked on Haswell
Patchwork writes: > == Series Details == > > Series: drm/i915: Fix system hang with EI UP masked on Haswell > URL : https://patchwork.freedesktop.org/series/22991/ > State : failure > > == Summary == > > Series 22991v1 drm/i915: Fix system hang with EI UP masked on Haswell > https://patchwork.freedesktop.org/api/1.0/series/22991/revisions/1/mbox/ > > Test gem_exec_flush: > Subgroup basic-batch-kernel-default-uc: > pass -> FAIL (fi-snb-2600) fdo#17 > Test gem_exec_suspend: > Subgroup basic-s4-devices: > pass -> DMESG-WARN (fi-kbl-7560u) fdo#100125 > Test kms_cursor_legacy: > Subgroup basic-flip-before-cursor-varying-size: > pass -> INCOMPLETE (fi-bxt-t5700) Patch is not affecting BXT so it has to be bug in drm-tip: https://bugs.freedesktop.org/show_bug.cgi?id=100706 -Mika > > fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17 > fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125 > > fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 > time:434s > fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 > time:429s > fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 > time:580s > fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 > time:507s > fi-bxt-t5700 total:206 pass:192 dwarn:0 dfail:0 fail:0 skip:13 > fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 > time:487s > fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 > time:482s > fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 > time:414s > fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 > time:405s > fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 > time:418s > fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 > time:488s > fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 > time:463s > fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 > time:455s > fi-kbl-7560u total:278 pass:267 dwarn:1 dfail:0 fail:0 skip:10 > time:565s > fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 > time:459s > fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 > time:574s > fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 > time:460s > fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 > time:491s > fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 > time:441s > fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 > time:533s > fi-snb-2600 total:278 pass:248 dwarn:0 dfail:0 fail:1 skip:29 > time:402s > > 6184edce6665aee9c9131149a7b9314a1313eaf9 drm-tip: 2017y-04m-13d-08h-27m-10s > UTC integration manifest > aee691a drm/i915: Fix system hang with EI UP masked on Haswell > > == Logs == > > For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4501/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/cma-helper: Return ENOENT for "no such gem obj"
All the error codes we (ab)use are strictly not the right ones (since
they're all for the vfs, and the only thing we're allowed to do from
an ioctl is EINVAL). But ENOENT is the common error code for failed to
look up an object throughout drm, so let's use it in the cma helpers,
too.
Cc: Laurent Pinchart
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_fb_cma_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
b/drivers/gpu/drm/drm_fb_cma_helper.c
index d2b77b02830d..53f9bdf470d7 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -189,7 +189,7 @@ struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct
drm_device *dev,
obj = drm_gem_object_lookup(file_priv, mode_cmd->handles[i]);
if (!obj) {
dev_err(dev->dev, "Failed to lookup GEM object\n");
- ret = -ENXIO;
+ ret = -ENOENT;
goto err_gem_object_put;
}
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix system hang with EI UP masked on Haswell
Chris Wilson writes:
> On Thu, Apr 13, 2017 at 02:15:27PM +0300, Mika Kuoppala wrote:
>> Previously with commit a9c1f90c8e17
>> ("drm/i915: Don't mask EI UP interrupt on IVB|SNB") certain,
>> seemingly unrelated bit (GEN6_PM_RP_UP_EI_EXPIRED) was needed
>> to be unmasked for IVB and SNB in order to prevent system hang
>> with chained batchbuffers.
>>
>> Our CI was seeing incomplete results with tests that used
>> chained batches and it was found out that HSW needs to have this
>> same bit unmasked to reliably survive chained batches.
>>
>> Always unmask GEN6_PM_RP_UP_EI_EXPIRED on Haswell to
>> prevent system hang with batch chaining.
>>
>> Testcase: igt/gem_exec_fence/nb-await-default
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100672
>> Cc: Chris Wilson
>> Cc: sta...@vger.kernel.org
>> Signed-off-by: Mika Kuoppala
>
> * facepalm.
>
> I am amazed that took so long for us to notice.
> Acked-by: Chris Wilson
>
Pushed to drm-intel-next-queued. Thanks.
-Mika
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/cma-helper: Return ENOENT for "no such gem obj"
Hi Daniel,
Thank you for the patch.
On Tuesday 18 Apr 2017 14:11:20 Daniel Vetter wrote:
> All the error codes we (ab)use are strictly not the right ones (since
> they're all for the vfs, and the only thing we're allowed to do from
> an ioctl is EINVAL). But ENOENT is the common error code for failed to
> look up an object throughout drm, so let's use it in the cma helpers,
> too.
Regardless of which is best, it's true that ENOENT is used through the DRM
code, so
Reviewed-by: Laurent Pinchart
Someone should however mention that this changes the userspace API. I'll let
you decide whether to ignore that comment :-)
> Cc: Laurent Pinchart
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_fb_cma_helper.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> b/drivers/gpu/drm/drm_fb_cma_helper.c index d2b77b02830d..53f9bdf470d7
> 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -189,7 +189,7 @@ struct drm_framebuffer
> *drm_fb_cma_create_with_funcs(struct drm_device *dev, obj =
> drm_gem_object_lookup(file_priv, mode_cmd->handles[i]);
> if (!obj) {
> dev_err(dev->dev, "Failed to lookup GEM object\n");
> - ret = -ENXIO;
> + ret = -ENOENT;
> goto err_gem_object_put;
> }
--
Regards,
Laurent Pinchart
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v2] igt/gem_exec_nop/headless: Verify GT performance in headless mode
On Tue, Apr 18, 2017 at 12:41:14PM +0100, Tvrtko Ursulin wrote:
> +static void headless(int fd, uint32_t handle)
> +{
> + const struct intel_execution_engine *e = &intel_execution_engines[0];
> + unsigned int nr_connected = 0;
> + drmModeConnector *connector;
> + drmModeRes *res;
> + double n[2];
> +
> + res = drmModeGetResources(fd);
> + igt_assert(res);
> +
> + /* require at least one connected connector for the test */
> + for (int i = 0; i < res->count_connectors; i++) {
> + connector = drmModeGetConnectorCurrent(fd, res->connectors[i]);
> + if (connector->connection == DRM_MODE_CONNECTED)
> + nr_connected++;
> + drmModeFreeConnector(connector);
> + }
> + igt_require(nr_connected > 0);
> +
> + /* set graphics mode to prevent blanking */
> + kmstest_set_vt_graphics_mode();
> +
> + /* benchmark nops */
> + n[0] = stable_nop_on_ring(fd, handle, e->exec_id | e->flags, 1, 5);
Minor, just use I915_EXEC_DEFAULT that has the semantics you want.
I think the test is interesting and further improvements can be made in
situ.
Reviewed-by: Chris Wilson
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/cma-helper: Return ENOENT for "no such gem obj"
== Series Details == Series: drm/cma-helper: Return ENOENT for "no such gem obj" URL : https://patchwork.freedesktop.org/series/23172/ State : failure == Summary == Series 23172v1 drm/cma-helper: Return ENOENT for "no such gem obj" https://patchwork.freedesktop.org/api/1.0/series/23172/revisions/1/mbox/ Test gem_exec_flush: Subgroup basic-batch-kernel-default-uc: pass -> FAIL (fi-snb-2600) fdo#17 Test gem_exec_suspend: Subgroup basic-s4-devices: dmesg-warn -> PASS (fi-kbl-7560u) fdo#100125 Test pm_rpm: Subgroup basic-rte: pass -> INCOMPLETE (fi-bsw-n3050) fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17 fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:429s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:425s fi-bsw-n3050 total:242 pass:207 dwarn:0 dfail:0 fail:0 skip:34 fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:517s fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:487s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:410s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:402s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:421s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:488s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:472s fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:458s fi-kbl-7560u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:594s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:454s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time:578s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:461s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:487s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:433s fi-snb-2600 total:278 pass:248 dwarn:0 dfail:0 fail:1 skip:29 time:407s fi-byt-n2820 failed to collect. IGT log at Patchwork_4511/fi-byt-n2820/igt.log fi-snb-2520m failed to collect. IGT log at Patchwork_4511/fi-snb-2520m/igt.log 4d374295ace9e57d83483341e2ad07cad5baf912 drm-tip: 2017y-04m-18d-10h-08m-05s UTC integration manifest 2cf2e53 drm/cma-helper: Return ENOENT for "no such gem obj" == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4511/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v3] lib/igt_kms: Force outputs to use full range RGB
In at least SKL and GLK (possibly other devices too), using a cursor
plane to scan out an fb might result in a different pipe crc than when
using a regular plane at the same position with the same fb while using
the CSC logic to limit the color range. The differences could be caused
by the cursor plane being limited to 8 bpc while the regular planes
support higher bit depths, leading to slightly different values to be
used internally. This is evidenced by the failures happening with
specific color values, 0.5 for example, but that's mostly speculation.
To avoid misterious failures caused by limited range rgb, force all
tests to use full range. It is still possible for tests to override this
if necessary.
v2: Add more details to the commit message.
v3: Force all tests to use full range.
Cc: Ville Syrjälä
Signed-off-by: Ander Conselvan de Oliveira
---
lib/igt_kms.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 5811414..9f72913 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1455,10 +1455,15 @@ static void igt_output_refresh(igt_output_t *output)
-1);
}
- if (output->config.connector)
+ if (output->config.connector) {
igt_atomic_fill_connector_props(display, output,
IGT_NUM_CONNECTOR_PROPS, igt_connector_prop_names);
+ kmstest_set_connector_broadcast_rgb(display->drm_fd,
+ output->config.connector,
+ BROADCAST_RGB_FULL);
+ }
+
if (output->use_override_mode)
output->config.default_mode = output->override_mode;
--
2.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/selftests: Allocate inode/file dynamically
Avoid having too large a stack by creating the fake struct inode/file on
the heap instead.
drivers/gpu/drm/i915/selftests/mock_drm.c: In function 'mock_file':
drivers/gpu/drm/i915/selftests/mock_drm.c:46:1: error: the frame size of 1328
bytes is larger than 1280 bytes [-Werror=frame-larger-than=]
drivers/gpu/drm/i915/selftests/mock_drm.c: In function 'mock_file_free':
drivers/gpu/drm/i915/selftests/mock_drm.c:54:1: error: the frame size of 1312
bytes is larger than 1280 bytes [-Werror=frame-larger-than=]
Reported-by: Arnd Bergmann
Fixes: 66d9cb5d805a ("drm/i915: Mock the GEM device for self-testing")
Signed-off-by: Chris Wilson
Cc: Joonas Lahtinen
Cc: Tvrtko Ursulin
Cc: Matthew Auld
Cc: Arnd Bergmann
---
drivers/gpu/drm/i915/selftests/mock_drm.c | 45 ++-
1 file changed, 32 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/selftests/mock_drm.c
b/drivers/gpu/drm/i915/selftests/mock_drm.c
index 113dec05c7dc..09c704153456 100644
--- a/drivers/gpu/drm/i915/selftests/mock_drm.c
+++ b/drivers/gpu/drm/i915/selftests/mock_drm.c
@@ -24,31 +24,50 @@
#include "mock_drm.h"
-static inline struct inode fake_inode(struct drm_i915_private *i915)
-{
- return (struct inode){ .i_rdev = i915->drm.primary->index };
-}
-
struct drm_file *mock_file(struct drm_i915_private *i915)
{
- struct inode inode = fake_inode(i915);
- struct file filp = {};
+ struct file *filp;
+ struct inode *inode;
struct drm_file *file;
int err;
- err = drm_open(&inode, &filp);
- if (unlikely(err))
- return ERR_PTR(err);
+ inode = kzalloc(sizeof(*inode), GFP_KERNEL);
+ if (!inode) {
+ err = -ENOMEM;
+ goto err;
+ }
+
+ inode->i_rdev = i915->drm.primary->index;
- file = filp.private_data;
+ filp = kzalloc(sizeof(*filp), GFP_KERNEL);
+ if (!filp) {
+ err = -ENOMEM;
+ goto err_inode;
+ }
+
+ err = drm_open(inode, filp);
+ if (err)
+ goto err_filp;
+
+ file = filp->private_data;
+ memset(&file->filp, POISON_INUSE, sizeof(file->filp));
file->authenticated = true;
+
+ kfree(filp);
+ kfree(inode);
return file;
+
+err_filp:
+ kfree(filp);
+err_inode:
+ kfree(inode);
+err:
+ return ERR_PTR(err);
}
void mock_file_free(struct drm_i915_private *i915, struct drm_file *file)
{
- struct inode inode = fake_inode(i915);
struct file filp = { .private_data = file };
- drm_release(&inode, &filp);
+ drm_release(NULL, &filp);
}
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/selftests: Allocate inode/file dynamically
== Series Details == Series: drm/i915/selftests: Allocate inode/file dynamically URL : https://patchwork.freedesktop.org/series/23183/ State : success == Summary == Series 23183v1 drm/i915/selftests: Allocate inode/file dynamically https://patchwork.freedesktop.org/api/1.0/series/23183/revisions/1/mbox/ fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:427s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:424s fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:578s fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time:540s fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:485s fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:480s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:408s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:410s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:423s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:479s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:474s fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:462s fi-kbl-7560u total:278 pass:267 dwarn:1 dfail:0 fail:0 skip:10 time:566s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:458s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time:571s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:459s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:494s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:432s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:533s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time:399s fi-bxt-j4205 failed to collect. IGT log at Patchwork_4512/fi-bxt-j4205/igt.log b15e4217c2dc477367d7239fc94d1c1e80698395 drm-tip: 2017y-04m-18d-12h-25m-18s UTC integration manifest 2ce8b29 drm/i915/selftests: Allocate inode/file dynamically == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4512/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 1/3] ACPI / bus: Introduce a list of ids for "always present" devices
On Tue, Apr 18, 2017 at 1:54 PM, Hans de Goede wrote:
> Several Cherry Trail devices (all of which ship with Windows 10) hide the
> LPSS PWM controller in ACPI, typically the _STA method looks like this:
>
> Method (_STA, 0, NotSerialized) // _STA: Status
> {
> If (OSID == One)
> {
> Return (Zero)
> }
>
> Return (0x0F)
> }
>
> Where OSID is some dark magic seen in all Cherry Trail ACPI tables making
> the machine behave differently depending on which OS it *thinks* it is
> booting, this gets set in a number of ways which we cannot control, on
> some newer machines it simple hardcoded to "One" aka win10.
>
> This causes the PWM controller to get hidden, which means Linux cannot
> control the backlight level on cht based tablets / laptops.
>
> Since loading the driver for this does no harm (the only in kernel user
> of it is the i915 driver, which will only use it when it needs it), this
> commit makes acpi_bus_get_status() always set status to ACPI_STA_DEFAULT
> for the 80862288 device, fixing the lack of backlight control.
>
> Signed-off-by: Hans de Goede
> ---
> Changes in v2:
> -Use pr_debug instead of ACPI_DEBUG_PRINT
> Changes in v3:
> -Un-inline acpi_set_device_status and do the always_present_device_ids
> table check inside the un-inlined version of it
> Changes in v4:
> -Use dev_info instead of pr_debug
> -Not only check for ACPI HID but also for CPU (SoC) model so as to not
> for devices present on other models then for which the quirk is intended and
> to avoid enabling unrelated ACPI devices which happen to use the same HID
> Changes in v5:
> -Only do the dev_info once per device (acpi_set_device_status gets called
> multiple times per device during boot)
> Changes in v6:
> -Allow specifying more then one CPU-model for a single HID
> -Not only match the HID but also the UID, like on Cherry Trail, on some Bay
> Trail Windows 10 tablets we need to enable the PWM controller to get working
> backlight even though _STA returns 0. The Bay Trail SoC has 2 PWM controllers
> and we only need the first one. UID matching will allows adding an entry for
> Bay Trail which only enables the first PWM controller
> ---
> drivers/acpi/bus.c | 65
> +
> include/acpi/acpi_bus.h | 6 +
> 2 files changed, 66 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index 34fbe02..eb30630 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -34,6 +34,8 @@
> #include
> #include
> #ifdef CONFIG_X86
> +#include
> +#include
> #include
> #endif
> #include
> @@ -132,6 +134,69 @@ int acpi_bus_get_status(struct acpi_device *device)
> }
> EXPORT_SYMBOL(acpi_bus_get_status);
>
> +#ifdef CONFIG_X86
> +/*
> + * Some ACPI devices are hidden (status == 0x0) in recent BIOS-es because
> + * some recent Windows drivers bind to one device but poke at multiple
> + * devices at the same time, so the others get hidden.
> + * We work around this by always reporting ACPI_STA_DEFAULT for these
> + * devices. Note this MUST only be done for devices where this is safe.
> + *
> + * This forcing of devices to be present is limited to specific CPU (SoC)
> + * models both to avoid potentially causing trouble on other models and
> + * because some HIDs are re-used on different SoCs for completely
> + * different devices.
> + */
> +struct always_present_device_id {
> + struct acpi_device_id hid[2];
> + struct x86_cpu_id cpu_ids[2];
This really is x86-specific, so it should go into somewhere like
arch/x86/kernel/acpi/ or drivers/acpi/x86/ (not present yet).
> + const char *uid;
> +};
> +
> +#define ICPU(model){ X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY, }
> +
> +#define ENTRY(hid, uid, cpu_models) { \
> + { { hid, }, {} }, \
> + { cpu_models, {} }, \
> + uid,\
> +}
> +
> +static const struct always_present_device_id always_present_device_ids[] = {
> + /*
> +* Cherry Trail PWM directly poked by GPU driver in win10,
> +* but Linux uses a separate PWM driver, harmless if not used.
> +*/
> + ENTRY("80862288", "1", ICPU(INTEL_FAM6_ATOM_AIRMONT)),
> +};
> +#endif
> +
> +void acpi_set_device_status(struct acpi_device *adev, u32 sta)
> +{
> + u32 *status = (u32 *)&adev->status;
> +#ifdef CONFIG_X86
> + u32 old_status = *status;
> + int i;
> +
> + /* acpi_match_device_ids checks status, so start with default */
> + *status = ACPI_STA_DEFAULT;
> + for (i = 0; i < ARRAY_SIZE(always_present_device_ids); i++) {
> + if (acpi_match_device_ids(adev,
> + always_present_device_ids[i].hid) == 0 &&
> + adev->pnp.unique_id &&
> +
Re: [Intel-gfx] [PATCH] drm/i915/selftests: Allocate inode/file dynamically
On Tue, Apr 18, 2017 at 3:12 PM, Chris Wilson wrote:
> Avoid having too large a stack by creating the fake struct inode/file on
> the heap instead.
>
> drivers/gpu/drm/i915/selftests/mock_drm.c: In function 'mock_file':
> drivers/gpu/drm/i915/selftests/mock_drm.c:46:1: error: the frame size of 1328
> bytes is larger than 1280 bytes [-Werror=frame-larger-than=]
> drivers/gpu/drm/i915/selftests/mock_drm.c: In function 'mock_file_free':
> drivers/gpu/drm/i915/selftests/mock_drm.c:54:1: error: the frame size of 1312
> bytes is larger than 1280 bytes [-Werror=frame-larger-than=]
>
> Reported-by: Arnd Bergmann
> Fixes: 66d9cb5d805a ("drm/i915: Mock the GEM device for self-testing")
> Signed-off-by: Chris Wilson
> Cc: Joonas Lahtinen
> Cc: Tvrtko Ursulin
> Cc: Matthew Auld
> Cc: Arnd Bergmann
This is clearly an improvement over the current state and it gets rid of
the warning, so I'm fine with this getting merged.
Acked-by: Arnd Bergmann
A nicer solution might be a wrapper around drm_open_helper() that
does not require a file pointer like this (completely untested, probably
wrong) patch would:
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index 3783b659cd38..4956c3945e7e 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -189,25 +189,38 @@ static int drm_cpu_valid(void)
*/
static int drm_open_helper(struct file *filp, struct drm_minor *minor)
{
- struct drm_device *dev = minor->dev;
struct drm_file *priv;
int ret;
if (filp->f_flags & O_EXCL)
return -EBUSY; /* No exclusive opens */
+
+ priv = drm_open_dev(minor);
+ ret = PTR_ERR_OR_ZERO(priv);
+ if (ret)
+ return ret;
+
+ filp->private_data = priv;
+ priv->filp = filp;
+}
+
+struct drm_file *drm_open_dev(struct drm_minor *minor)
+{
+ struct drm_device *dev = minor->dev;
+ struct drm_file *priv;
+ int ret;
+
if (!drm_cpu_valid())
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
if (dev->switch_power_state != DRM_SWITCH_POWER_ON &&
dev->switch_power_state != DRM_SWITCH_POWER_DYNAMIC_OFF)
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
DRM_DEBUG("pid = %d, minor = %d\n", task_pid_nr(current), minor->index);
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (!priv)
- return -ENOMEM;
+ return ERR_PTR(-ENOMEM);
- filp->private_data = priv;
- priv->filp = filp;
priv->pid = get_pid(task_pid(current));
priv->minor = minor;
@@ -268,7 +281,7 @@ static int drm_open_helper(struct file *filp,
struct drm_minor *minor)
}
#endif
- return 0;
+ return priv;
out_close:
if (dev->driver->postclose)
@@ -280,8 +293,7 @@ static int drm_open_helper(struct file *filp,
struct drm_minor *minor)
drm_gem_release(dev, priv);
put_pid(priv->pid);
kfree(priv);
- filp->private_data = NULL;
- return ret;
+ return ERR_PTR(ret);
}
static void drm_events_release(struct drm_file *file_priv)
If anyone cares more about that than I do, this could be a follow-up patch.
Arnd
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] freedesktop bug id: 100548, bisected to sched/clock commit
> -Original Message-
> From: Peter Zijlstra [mailto:pet...@infradead.org]
> Sent: Thursday, April 13, 2017 4:24 PM
> To: Martin Peres
> Cc: Lofstedt, Marta ;
> pasha.tatas...@oracle.com; intel-gfx@lists.freedesktop.org; Thomas
> Gleixner
> Subject: Re: freedesktop bug id: 100548, bisected to sched/clock commit
>
> On Thu, Apr 13, 2017 at 03:30:25PM +0300, Martin Peres wrote:
> > On 13/04/17 14:48, Peter Zijlstra wrote:
> > > On Wed, Apr 12, 2017 at 05:49:53PM +0300, Martin Peres wrote:
> > >
> > > > Good to know. Is there a way to disable this behaviour, as a
> > > > workaround for our CI system until a proper fix lands? We already
> > > > pushed locally the revert for this patch, but that may affect
> > > > other platforms which do not exhibit the problem.
> > >
> > > Blergh, so the patch is correct, but the __gtod_offset calculation
> > > is fed with absolute crap numbers due to 'circumstances' and then
> > > using it ends up being worse than not using it.
> >
> > Thanks for taking this bug seriously!
>
> So I've not actually dug out a Core2 machine, so have only tested this by
> poking random values into the TSC MSR on an otherwise 'good' machine.
>
> Could you give it a go to see if it works for you?
Sorry Peter, I still see regression on the Core2 machine, with your patch.
/Marta
>
> Thomas, how much hate?
>
> ---
> Subject: sched/clock,x86/tsc: Improve clock continuity for stable->unstable
> transition
> From: Peter Zijlstra
> Date: Thu Apr 13 14:56:44 CEST 2017
>
> Marta reported that commit:
>
> 7b09cc5a9deb ("sched/clock: Fix broken stable to unstable transfer")
>
> Appeared to have broken things on a Core2Duo machine. While that patch is
> in fact correct, it exposes a problem with commit:
>
> 5680d8094ffa ("sched/clock: Provide better clock continuity")
>
> Where we hoped that TSC would not make big jumps after SMP bringup. Of
> course, TSC needs to prove us wrong. Because Core2 comes up with a semi-
> stable TSC and only goes funny once we probe the idle drivers, because
> Core2 stops TSC on idle.
>
> Now we could of course delay the final switch to stable longer, but it would
> be better to entirely remove the assumption that TSC doesn't make big
> jumps and improve things all-round.
>
> So instead we have the clocksource watchdog call a special function when it
> finds the TSC is still good (there's a race, it could've gotten bad between us
> determining it's still good and calling our function, do we care?).
>
> This function then updates the __gtod_offset using sane values, which is the
> value needed for clock continuity when being marked unstable.
>
> Cc: Pavel Tatashin
> Cc: Martin Peres
> Reported-by: "Lofstedt, Marta"
> Fixes: 5680d8094ffa ("sched/clock: Provide better clock continuity")
> Signed-off-by: Peter Zijlstra (Intel)
> ---
> arch/x86/kernel/tsc.c | 12 ++
> include/linux/clocksource.h |1
> include/linux/sched/clock.h |2 -
> kernel/sched/clock.c| 50 --
> --
> kernel/time/clocksource.c |3 ++
> 5 files changed, 45 insertions(+), 23 deletions(-)
>
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -374,6 +374,8 @@ static int __init tsc_setup(char *str)
> tsc_clocksource_reliable = 1;
> if (!strncmp(str, "noirqtime", 9))
> no_sched_irq_time = 1;
> + if (!strcmp(str, "unstable"))
> + mark_tsc_unstable("boot parameter");
> return 1;
> }
>
> @@ -1127,6 +1129,15 @@ static void tsc_cs_mark_unstable(struct
> pr_info("Marking TSC unstable due to clocksource
> watchdog\n"); }
>
> +static void tsc_cs_tick_stable(struct clocksource *cs) {
> + if (tsc_unstable)
> + return;
> +
> + if (using_native_sched_clock())
> + sched_clock_tick_stable();
> +}
> +
> /*
> * .mask MUST be CLOCKSOURCE_MASK(64). See comment above
> read_tsc()
> */
> @@ -1140,6 +1151,7 @@ static struct clocksource clocksource_ts
> .archdata = { .vclock_mode = VCLOCK_TSC },
> .resume = tsc_resume,
> .mark_unstable =
> tsc_cs_mark_unstable,
> + .tick_stable= tsc_cs_tick_stable,
> };
>
> void mark_tsc_unstable(char *reason)
> --- a/include/linux/clocksource.h
> +++ b/include/linux/clocksource.h
> @@ -96,6 +96,7 @@ struct clocksource {
> void (*suspend)(struct clocksource *cs);
> void (*resume)(struct clocksource *cs);
> void (*mark_unstable)(struct clocksource *cs);
> + void (*tick_stable)(struct clocksource *cs);
>
> /* private: */
> #ifdef CONFIG_CLOCKSOURCE_WATCHDOG
> --- a/include/linux/sched/clock.h
> +++ b/include/linux/sched/clock.h
> @@ -63,8 +63,8 @@ extern void clear_sched_clock_stable(voi
> */
> extern u64 __sched_clock_offset;
>
> -
> extern void sched_clock_tick(void);
> +extern void sched_clock_tick_stable(void);
> extern void sched_clock_idle_sleep_event(void);
> ext
Re: [Intel-gfx] [PATCH v2 2/2] drm/i915/scheduler: add gvt notification for guc
Hi Chris, Ping for your feedback on my comments. Please help to make it to move forward. :) Thanks Chuanxiao > -Original Message- > From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On > Behalf Of Dong, Chuanxiao > Sent: Thursday, April 13, 2017 7:03 PM > To: Chris Wilson; intel-gfx@lists.freedesktop.org > Cc: Zheng, Xiao; Tian, Kevin; joonas.lahti...@linux.intel.com; intel-gvt- > d...@lists.freedesktop.org; Wang, Zhi A > Subject: RE: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification for guc > > > -Original Message- > > From: intel-gvt-dev > > [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On Behalf Of > > Dong, Chuanxiao > > Sent: Wednesday, April 12, 2017 5:12 PM > > To: Chris Wilson > > Cc: Tian, Kevin ; > > intel-gvt-...@lists.freedesktop.org; > > intel-gfx@lists.freedesktop.org; joonas.lahti...@linux.intel.com; > > Zheng, Xiao ; Wang, Zhi A > > Subject: RE: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification > > for guc > > > > > > > > > -Original Message- > > > From: intel-gvt-dev > > > [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On Behalf Of > > > Chris Wilson > > > Sent: Wednesday, April 12, 2017 4:22 PM > > > To: Dong, Chuanxiao > > > Cc: Tian, Kevin ; > > > intel-gvt-...@lists.freedesktop.org; > > > intel-gfx@lists.freedesktop.org; joonas.lahti...@linux.intel.com; > > > Zheng, Xiao ; Wang, Zhi A > > > > > > Subject: Re: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification > > > for guc > > > > > > On Mon, Apr 10, 2017 at 02:40:24AM +, Dong, Chuanxiao wrote: > > > > > -Original Message- > > > > > From: intel-gvt-dev > > > > > [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On Behalf > > > > > Of Dong, Chuanxiao > > > > > Sent: Thursday, April 6, 2017 11:19 PM > > > > > To: Chris Wilson > > > > > Cc: Tian, Kevin ; > > > > > intel-gvt-...@lists.freedesktop.org; > > > > > intel-gfx@lists.freedesktop.org; > > > > > joonas.lahti...@linux.intel.com; Zheng, Xiao > > > > > ; Wang, Zhi A > > > > > Subject: RE: [PATCH v2 2/2] drm/i915/scheduler: add gvt > > > > > notification for guc > > > > > > > > > > > > > > > > > > > > > -Original Message- > > > > > > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > > > > > > Sent: Thursday, April 6, 2017 11:07 PM > > > > > > To: Dong, Chuanxiao > > > > > > Cc: intel-gfx@lists.freedesktop.org; > > > > > > intel-gvt-...@lists.freedesktop.org; > > > > > > Zheng, Xiao; Tian, Kevin; joonas.lahti...@linux.intel.com; > > > > > > Wang, Zhi A > > > > > > Subject: Re: [PATCH v2 2/2] drm/i915/scheduler: add gvt > > > > > > notification for guc > > > > > > > > > > > > On Thu, Apr 06, 2017 at 02:49:54PM +, Dong, Chuanxiao wrote: > > > > > > > > > > > > > > > > > > > > > > -Original Message- > > > > > > > > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > > > > > > > > Sent: Thursday, April 6, 2017 10:19 PM > > > > > > > > To: Dong, Chuanxiao > > > > > > > > Cc: intel-gfx@lists.freedesktop.org; > > > > > > > > intel-gvt-...@lists.freedesktop.org; > > > > > > > > Zheng, Xiao; Tian, Kevin; joonas.lahti...@linux.intel.com > > > > > > > > Subject: Re: [PATCH v2 2/2] drm/i915/scheduler: add gvt > > > > > > > > notification for guc > > > > > > > > > > > > > > > > On Thu, Apr 06, 2017 at 02:05:15PM +, Dong, Chuanxiao > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > -Original Message- > > > > > > > > > > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > > > > > > > > > > Sent: Thursday, April 6, 2017 9:32 PM > > > > > > > > > > To: Dong, Chuanxiao > > > > > > > > > > Cc: intel-gfx@lists.freedesktop.org; > > > > > > > > > > intel-gvt-...@lists.freedesktop.org; > > > > > > > > > > Zheng, Xiao; Tian, Kevin; > > > > > > > > > > joonas.lahti...@linux.intel.com > > > > > > > > > > Subject: Re: [PATCH v2 2/2] drm/i915/scheduler: add > > > > > > > > > > gvt notification for guc > > > > > > > > > > > > > > > > > > > > On Tue, Mar 28, 2017 at 05:38:41PM +0800, Chuanxiao > > > > > > > > > > Dong > > > wrote: > > > > > > > > > > > GVT request needs a manual mmio load/restore. Before > > > > > > > > > > > GuC submit a request, send notification to gvt for > > > > > > > > > > > mmio > > loading. > > > > > > > > > > > And after the GuC finished this GVT request, notify > > > > > > > > > > > gvt again for > > > > > > mmio restore. > > > > > > > > > > > This follows the usage when using execlists submission. > > > > > > > > > > > > > > > > > > > > > > Cc: xiao.zh...@intel.com > > > > > > > > > > > Cc: kevin.t...@intel.com > > > > > > > > > > > Cc: joonas.lahti...@linux.intel.com > > > > > > > > > > > Cc: ch...@chris-wilson.co.uk > > > > > > > > > > > Signed-off-by: Chuanxiao Dong > > > > > > > > > > > > > > > > > > > > > > --- > > > > > > > > > > > drivers/gpu/drm/i915/i915_guc_submission.c | 4 > > > > > > > > > > > drivers/gpu/drm/i915/intel_gvt.h | 12 > > > > > > > > > > > > drivers/gpu/drm/i915/intel_lrc.
Re: [Intel-gfx] [PATCH 16/22] xen-blkfront: Make use of the new sg_map helper function
On Tue, Apr 18, 2017 at 02:13:59PM +, David Laight wrote:
> From: Logan Gunthorpe
> > Sent: 13 April 2017 23:05
> > Straightforward conversion to the new helper, except due to
> > the lack of error path, we have to warn if unmapable memory
> > is ever present in the sgl.
Interesting that you didn't CC any of the maintainers. Could you
do that in the future please?
> >
> > Signed-off-by: Logan Gunthorpe
> > ---
> > drivers/block/xen-blkfront.c | 33 +++--
> > 1 file changed, 27 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> > index 5067a0a..7dcf41d 100644
> > --- a/drivers/block/xen-blkfront.c
> > +++ b/drivers/block/xen-blkfront.c
> > @@ -807,8 +807,19 @@ static int blkif_queue_rw_req(struct request *req,
> > struct blkfront_ring_info *ri
> > BUG_ON(sg->offset + sg->length > PAGE_SIZE);
> >
> > if (setup.need_copy) {
> > - setup.bvec_off = sg->offset;
> > - setup.bvec_data = kmap_atomic(sg_page(sg));
> > + setup.bvec_off = 0;
> > + setup.bvec_data = sg_map(sg, SG_KMAP_ATOMIC);
> > + if (IS_ERR(setup.bvec_data)) {
> > + /*
> > +* This should really never happen unless
> > +* the code is changed to use memory that is
> > +* not mappable in the sg. Seeing there is a
> > +* questionable error path out of here,
> > +* we WARN.
> > +*/
> > + WARN(1, "Non-mappable memory used in sg!");
> > + return 1;
> > + }
> ...
>
> Perhaps add a flag to mark failure as 'unexpected' and trace (and panic?)
> inside sg_map().
>
> David
>
>
> ___
> Linux-nvdimm mailing list
> linux-nvd...@lists.01.org
> https://lists.01.org/mailman/listinfo/linux-nvdimm
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/cma-helper: Return ENOENT for "no such gem obj"
On Tue, Apr 18, 2017 at 03:29:29PM +0300, Laurent Pinchart wrote:
> Hi Daniel,
>
> Thank you for the patch.
>
> On Tuesday 18 Apr 2017 14:11:20 Daniel Vetter wrote:
> > All the error codes we (ab)use are strictly not the right ones (since
> > they're all for the vfs, and the only thing we're allowed to do from
> > an ioctl is EINVAL). But ENOENT is the common error code for failed to
> > look up an object throughout drm, so let's use it in the cma helpers,
> > too.
>
> Regardless of which is best, it's true that ENOENT is used through the DRM
> code, so
>
> Reviewed-by: Laurent Pinchart
>
> Someone should however mention that this changes the userspace API. I'll let
> you decide whether to ignore that comment :-)
Yeah, let's make sure we don't break any existing userspaces (a la pulseaudio).
Sean
>
> > Cc: Laurent Pinchart
> > Signed-off-by: Daniel Vetter
> > ---
> > drivers/gpu/drm/drm_fb_cma_helper.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> > b/drivers/gpu/drm/drm_fb_cma_helper.c index d2b77b02830d..53f9bdf470d7
> > 100644
> > --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> > @@ -189,7 +189,7 @@ struct drm_framebuffer
> > *drm_fb_cma_create_with_funcs(struct drm_device *dev, obj =
> > drm_gem_object_lookup(file_priv, mode_cmd->handles[i]);
> > if (!obj) {
> > dev_err(dev->dev, "Failed to lookup GEM object\n");
> > - ret = -ENXIO;
> > + ret = -ENOENT;
> > goto err_gem_object_put;
> > }
>
> --
> Regards,
>
> Laurent Pinchart
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 16/22] xen-blkfront: Make use of the new sg_map helper function
On 18/04/17 08:27 AM, Konrad Rzeszutek Wilk wrote:
> Interesting that you didn't CC any of the maintainers. Could you
> do that in the future please?
Please read the cover letter. The distribution list for the patchset
would have been way too large to cc every maintainer (even as limited as
it was, I had mailing lists yelling at me). My plan was to get buy in
for the first patch, get it merged and resend the rest independently to
their respective maintainers. Of course, though, I'd be open to other
suggestions.
>>>
>>> Signed-off-by: Logan Gunthorpe
>>> ---
>>> drivers/block/xen-blkfront.c | 33 +++--
>>> 1 file changed, 27 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
>>> index 5067a0a..7dcf41d 100644
>>> --- a/drivers/block/xen-blkfront.c
>>> +++ b/drivers/block/xen-blkfront.c
>>> @@ -807,8 +807,19 @@ static int blkif_queue_rw_req(struct request *req,
>>> struct blkfront_ring_info *ri
>>> BUG_ON(sg->offset + sg->length > PAGE_SIZE);
>>>
>>> if (setup.need_copy) {
>>> - setup.bvec_off = sg->offset;
>>> - setup.bvec_data = kmap_atomic(sg_page(sg));
>>> + setup.bvec_off = 0;
>>> + setup.bvec_data = sg_map(sg, SG_KMAP_ATOMIC);
>>> + if (IS_ERR(setup.bvec_data)) {
>>> + /*
>>> +* This should really never happen unless
>>> +* the code is changed to use memory that is
>>> +* not mappable in the sg. Seeing there is a
>>> +* questionable error path out of here,
>>> +* we WARN.
>>> +*/
>>> + WARN(1, "Non-mappable memory used in sg!");
>>> + return 1;
>>> + }
>> ...
>>
>> Perhaps add a flag to mark failure as 'unexpected' and trace (and panic?)
>> inside sg_map().
Thanks, that's a good suggestion. I'll make the change for v2.
Logan
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/22] drm/i915: Make use of the new sg_map helper function
On 18/04/17 12:44 AM, Daniel Vetter wrote: > On Thu, Apr 13, 2017 at 04:05:18PM -0600, Logan Gunthorpe wrote: >> This is a single straightforward conversion from kmap to sg_map. >> >> Signed-off-by: Logan Gunthorpe > > Acked-by: Daniel Vetter > > Probably makes sense to merge through some other tree, but please be aware > of the considerable churn rate in i915 (i.e. make sure your tree is in > linux-next before you send a pull request for this). Plane B would be to > get the prep patch in first and then merge the i915 conversion one kernel > release later. Yes, as per what I said in my cover letter, I was leaning towards a "Plan B" style approach. Logan ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 16/22] xen-blkfront: Make use of the new sg_map helper function
On Tue, Apr 18, 2017 at 09:42:20AM -0600, Logan Gunthorpe wrote:
>
>
> On 18/04/17 08:27 AM, Konrad Rzeszutek Wilk wrote:
> > Interesting that you didn't CC any of the maintainers. Could you
> > do that in the future please?
>
> Please read the cover letter. The distribution list for the patchset
> would have been way too large to cc every maintainer (even as limited as
> it was, I had mailing lists yelling at me). My plan was to get buy in
I am not sure if you know, but you can add on each patch the respective
maintainer via 'CC'. That way you can have certain maintainers CCed only
on the subsystems they cover. You put it after (or before) your SoB and
git send-email happilly picks it up.
It does mean that for every patch you have to run something like this:
$ more add_cc
#!/bin/bash
git diff HEAD^.. > /tmp/a
echo "---"
scripts/get_maintainer.pl --no-l /tmp/a | while read file
do
echo "Cc: $file"
done
Or such.
> for the first patch, get it merged and resend the rest independently to
> their respective maintainers. Of course, though, I'd be open to other
> suggestions.
>
> >>>
> >>> Signed-off-by: Logan Gunthorpe
> >>> ---
> >>> drivers/block/xen-blkfront.c | 33 +++--
> >>> 1 file changed, 27 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> >>> index 5067a0a..7dcf41d 100644
> >>> --- a/drivers/block/xen-blkfront.c
> >>> +++ b/drivers/block/xen-blkfront.c
> >>> @@ -807,8 +807,19 @@ static int blkif_queue_rw_req(struct request *req,
> >>> struct blkfront_ring_info *ri
> >>> BUG_ON(sg->offset + sg->length > PAGE_SIZE);
> >>>
> >>> if (setup.need_copy) {
> >>> - setup.bvec_off = sg->offset;
> >>> - setup.bvec_data = kmap_atomic(sg_page(sg));
> >>> + setup.bvec_off = 0;
> >>> + setup.bvec_data = sg_map(sg, SG_KMAP_ATOMIC);
> >>> + if (IS_ERR(setup.bvec_data)) {
> >>> + /*
> >>> + * This should really never happen unless
> >>> + * the code is changed to use memory that is
> >>> + * not mappable in the sg. Seeing there is a
> >>> + * questionable error path out of here,
> >>> + * we WARN.
> >>> + */
> >>> + WARN(1, "Non-mappable memory used in sg!");
> >>> + return 1;
> >>> + }
> >> ...
> >>
> >> Perhaps add a flag to mark failure as 'unexpected' and trace (and panic?)
> >> inside sg_map().
>
> Thanks, that's a good suggestion. I'll make the change for v2.
>
> Logan
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] freedesktop bug id: 100548, bisected to sched/clock commit
On Tue, Apr 18, 2017 at 02:10:07PM +, Lofstedt, Marta wrote: > Sorry Peter, I still see regression on the Core2 machine, with your patch. > Blergh, ok. I'll see if I can dig out an actual Core2 machine somewhere. I should have enough parts about. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/cma-helper: Return ENOENT for "no such gem obj"
On Tue, Apr 18, 2017 at 10:40:21AM -0400, Sean Paul wrote:
> On Tue, Apr 18, 2017 at 03:29:29PM +0300, Laurent Pinchart wrote:
> > Hi Daniel,
> >
> > Thank you for the patch.
> >
> > On Tuesday 18 Apr 2017 14:11:20 Daniel Vetter wrote:
> > > All the error codes we (ab)use are strictly not the right ones (since
> > > they're all for the vfs, and the only thing we're allowed to do from
> > > an ioctl is EINVAL). But ENOENT is the common error code for failed to
> > > look up an object throughout drm, so let's use it in the cma helpers,
> > > too.
> >
> > Regardless of which is best, it's true that ENOENT is used through the DRM
> > code, so
> >
> > Reviewed-by: Laurent Pinchart
> >
> > Someone should however mention that this changes the userspace API. I'll
> > let
> > you decide whether to ignore that comment :-)
>
> Yeah, let's make sure we don't break any existing userspaces (a la
> pulseaudio).
Good point, but I think this is safe. I augmented the commit message and
merged the patch with Sean's irc r-b (plus Laurent's r-b from here).
-Daniel
>
> Sean
>
> >
> > > Cc: Laurent Pinchart
> > > Signed-off-by: Daniel Vetter
> > > ---
> > > drivers/gpu/drm/drm_fb_cma_helper.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> > > b/drivers/gpu/drm/drm_fb_cma_helper.c index d2b77b02830d..53f9bdf470d7
> > > 100644
> > > --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> > > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> > > @@ -189,7 +189,7 @@ struct drm_framebuffer
> > > *drm_fb_cma_create_with_funcs(struct drm_device *dev, obj =
> > > drm_gem_object_lookup(file_priv, mode_cmd->handles[i]);
> > > if (!obj) {
> > > dev_err(dev->dev, "Failed to lookup GEM object\n");
> > > - ret = -ENXIO;
> > > + ret = -ENOENT;
> > > goto err_gem_object_put;
> > > }
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
> >
> > ___
> > dri-devel mailing list
> > dri-de...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 16/22] xen-blkfront: Make use of the new sg_map helper function
On 18/04/17 09:50 AM, Konrad Rzeszutek Wilk wrote: > I am not sure if you know, but you can add on each patch the respective > maintainer via 'CC'. That way you can have certain maintainers CCed only > on the subsystems they cover. You put it after (or before) your SoB and > git send-email happilly picks it up. Yes, but I've seen some maintainers complain when they receive a patch with no context (ie. cover letter and first patch). So I chose to do it this way. I expect in this situation, no matter what you do, someone is going to complain about the approach chosen. Thanks anyway for the tip. Logan ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 1/2] drm/i915: Engine discovery uAPI
From: Tvrtko Ursulin
Engine discovery uAPI allows userspace to probe for engine
configuration and features without needing to maintain the
internal PCI id based database.
This enables removal of code duplications across userspace
components.
Probing is done via the new DRM_IOCTL_I915_GEM_ENGINE_INFO ioctl
which returns the number and information on the specified engine
class.
Currently only general engine configuration and HEVC feature of
the VCS engine can be probed but the uAPI is designed to be
generic and extensible.
Code is based almost exactly on the earlier proposal on the
topic by Jon Bloomfield. Engine class and instance refactoring
made recently by Daniele Ceraolo Spurio enabled this to be
implemented in an elegant fashion.
To probe configuration userspace sets the engine class it wants
to query (struct drm_i915_gem_engine_info) and provides an array
of drm_i915_engine_info structs which will be filled in by the
driver. Userspace also has to tell i915 how many elements are in
the array, and the driver will report back the total number of
engine instances in any case.
Signed-off-by: Tvrtko Ursulin
Cc: Ben Widawsky
Cc: Chris Wilson
Cc: Daniel Vetter
Cc: Joonas Lahtinen
Cc: Jon Bloomfield
Cc: Daniel Charles
Cc: "Rogozhkin, Dmitry V"
Cc: Oscar Mateo
Cc: "Gong, Zhipeng"
Cc: intel-vaapi-me...@lists.01.org
Cc: mesa-...@lists.freedesktop.org
---
drivers/gpu/drm/i915/i915_drv.c| 1 +
drivers/gpu/drm/i915/i915_drv.h| 3 ++
drivers/gpu/drm/i915/intel_engine_cs.c | 59 ++
include/uapi/drm/i915_drm.h| 40 +++
4 files changed, 103 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index cc7393e65e99..b720c9468adf 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2607,6 +2607,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM,
i915_gem_context_getparam_ioctl, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_SETPARAM,
i915_gem_context_setparam_ioctl, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl,
DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(I915_GEM_ENGINE_INFO, i915_gem_engine_info_ioctl,
DRM_RENDER_ALLOW),
};
static struct drm_driver driver = {
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 357b6c6c2f04..6eed0e854561 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3547,6 +3547,9 @@ i915_gem_context_lookup_timeline(struct i915_gem_context
*ctx,
int i915_perf_open_ioctl(struct drm_device *dev, void *data,
struct drm_file *file);
+int i915_gem_engine_info_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file);
+
/* i915_gem_evict.c */
int __must_check i915_gem_evict_something(struct i915_address_space *vm,
u64 min_size, u64 alignment,
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c
b/drivers/gpu/drm/i915/intel_engine_cs.c
index 402769d9d840..6deaffd34ae0 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -25,6 +25,7 @@
#include "i915_drv.h"
#include "intel_ringbuffer.h"
#include "intel_lrc.h"
+#include
struct engine_class_info {
const char *name;
@@ -1189,6 +1190,64 @@ void intel_engines_reset_default_submission(struct
drm_i915_private *i915)
engine->set_default_submission(engine);
}
+u8 user_class_map[DRM_I915_ENGINE_CLASS_MAX] = {
+ [DRM_I915_ENGINE_CLASS_OTHER] = OTHER_CLASS,
+ [DRM_I915_ENGINE_CLASS_RENDER] = RENDER_CLASS,
+ [DRM_I915_ENGINE_CLASS_COPY] = COPY_ENGINE_CLASS,
+ [DRM_I915_ENGINE_CLASS_VIDEO_DECODE] = VIDEO_DECODE_CLASS,
+ [DRM_I915_ENGINE_CLASS_VIDEO_ENHANCE] = VIDEO_ENHANCEMENT_CLASS,
+};
+
+int i915_gem_engine_info_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file)
+{
+ struct drm_i915_private *i915 = to_i915(dev);
+ struct drm_i915_gem_engine_info *args = data;
+ enum drm_i915_gem_engine_class engine_class = args->engine_class;
+ struct drm_i915_engine_info __user *user_info =
+ u64_to_user_ptr(args->info_ptr);
+ struct drm_i915_engine_info info;
+ struct intel_engine_cs *engine;
+ enum intel_engine_id id;
+ u8 class;
+
+ if (args->rsvd)
+ return -EINVAL;
+
+ switch (engine_class) {
+ case DRM_I915_ENGINE_CLASS_OTHER:
+ case DRM_I915_ENGINE_CLASS_RENDER:
+ case DRM_I915_ENGINE_CLASS_COPY:
+ case DRM_I915_ENGINE_CLASS_VIDEO_DECODE:
+ case DRM_I915_ENGINE_CLASS_VIDEO_ENHANCE:
+ if (engine_class >= DRM_I915_ENGINE_CLASS_MAX)
+ return -EINVAL;
+ class = user_class_map[
[Intel-gfx] [RFC 2/2] drm/i915: Select engines via class and instance in execbuffer2
From: Tvrtko Ursulin
Building on top of the previous patch which exported the concept
of engine classes and instances, we can also use this instead of
the current awkward engine selection uAPI.
This is primarily interesting for the VCS engine selection which
is a) currently done via disjoint set of flags, and b) the
current I915_EXEC_BSD flags has different semantics depending on
the underlying hardware which is bad.
Proposed idea here is to reserve 16-bits of flags, to pass in
the engine class and instance (8 bits each), and a new flag
named I915_EXEC_CLASS_INSTACE to tell the kernel this new engine
selection API is in use.
The new uAPI also removes access to the weak VCS engine
balancing as currently existing in the driver.
Example usage to send a command to VCS0:
eb.flags = i915_execbuffer2_engine(DRM_I915_ENGINE_CLASS_VIDEO_DECODE, 0);
Or to send a command to VCS1:
eb.flags = i915_execbuffer2_engine(DRM_I915_ENGINE_CLASS_VIDEO_DECODE, 1);
Signed-off-by: Tvrtko Ursulin
Cc: Ben Widawsky
Cc: Chris Wilson
Cc: Daniel Vetter
Cc: Joonas Lahtinen
Cc: Jon Bloomfield
Cc: Daniel Charles
Cc: "Rogozhkin, Dmitry V"
Cc: Oscar Mateo
Cc: "Gong, Zhipeng"
Cc: intel-vaapi-me...@lists.01.org
Cc: mesa-...@lists.freedesktop.org
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 36 ++
include/uapi/drm/i915_drm.h| 7 +-
2 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index af1965774e7b..7fc92ec839a1 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1153,6 +1153,10 @@ i915_gem_check_execbuffer(struct
drm_i915_gem_execbuffer2 *exec)
if (exec->flags & __I915_EXEC_UNKNOWN_FLAGS)
return false;
+ if ((exec->flags & I915_EXEC_CLASS_INSTANCE) &&
+ (exec->flags & I915_EXEC_RING_MASK))
+ return false;
+
/* Kernel clipping was a DRI1 misfeature */
if (exec->num_cliprects || exec->cliprects_ptr)
return false;
@@ -1492,6 +1496,35 @@ gen8_dispatch_bsd_engine(struct drm_i915_private
*dev_priv,
return file_priv->bsd_engine;
}
+extern u8 user_class_map[DRM_I915_ENGINE_CLASS_MAX];
+
+static struct intel_engine_cs *
+eb_select_engine_class_instance(struct drm_i915_private *i915,
+ struct drm_i915_gem_execbuffer2 *args)
+{
+ struct intel_engine_cs *engine;
+ enum intel_engine_id id;
+ u16 class_instance;
+ u8 user_class, class, instance;
+
+ class_instance = (args->flags & I915_EXEC_CLASS_INSTANCE_MASK) >>
+I915_EXEC_CLASS_INSTANCE_SHIFT;
+
+ user_class = class_instance >> 8;
+ instance = class_instance & 0xff;
+
+ if (user_class >= DRM_I915_ENGINE_CLASS_MAX)
+ return NULL;
+ class = user_class_map[user_class];
+
+ for_each_engine(engine, i915, id) {
+ if (engine->class == class && engine->instance == instance)
+ return engine;
+ }
+
+ return NULL;
+}
+
#define I915_USER_RINGS (4)
static const enum intel_engine_id user_ring_map[I915_USER_RINGS + 1] = {
@@ -1510,6 +1543,9 @@ eb_select_engine(struct drm_i915_private *dev_priv,
unsigned int user_ring_id = args->flags & I915_EXEC_RING_MASK;
struct intel_engine_cs *engine;
+ if (args->flags & I915_EXEC_CLASS_INSTANCE)
+ return eb_select_engine_class_instance(dev_priv, args);
+
if (user_ring_id > I915_USER_RINGS) {
DRM_DEBUG("execbuf with unknown ring: %u\n", user_ring_id);
return NULL;
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 6058596a9f33..727a6dc4b029 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -906,7 +906,12 @@ struct drm_i915_gem_execbuffer2 {
*/
#define I915_EXEC_FENCE_OUT(1<<17)
-#define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_FENCE_OUT<<1))
+#define I915_EXEC_CLASS_INSTANCE (1<<18)
+
+#define I915_EXEC_CLASS_INSTANCE_SHIFT (19)
+#define I915_EXEC_CLASS_INSTANCE_MASK (0x <<
I915_EXEC_CLASS_INSTANCE_SHIFT)
+
+#define __I915_EXEC_UNKNOWN_FLAGS (-(35 << 1))
#define I915_EXEC_CONTEXT_ID_MASK (0x)
#define i915_execbuffer2_set_context_id(eb2, context) \
--
2.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 0/2] New engine discovery and execbuffer2 engine selection uAPI
From: Tvrtko Ursulin Inspired by the recent introduction of the engine class and instance concept, and a chat with Chris Wilson about a potential unification of PCI id based device discovery across multiple userspace components, I have cooked up two patches to gather some opinions on whether this sort of uAPI would be interesting for a wider audience. First patch is basically and old idea by Jon Bloomfield on how to do engine discovery, while the second one is just a quick hack to show how a similar approach could work for execbuffer2. Both patches are compile tested only and intended only to start a discussion. Tvrtko Ursulin (2): drm/i915: Engine discovery uAPI drm/i915: Select engines via class and instance in execbuffer2 drivers/gpu/drm/i915/i915_drv.c| 1 + drivers/gpu/drm/i915/i915_drv.h| 3 ++ drivers/gpu/drm/i915/i915_gem_execbuffer.c | 36 ++ drivers/gpu/drm/i915/intel_engine_cs.c | 59 ++ include/uapi/drm/i915_drm.h| 47 +++- 5 files changed, 145 insertions(+), 1 deletion(-) -- 2.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for New engine discovery and execbuffer2 engine selection uAPI
== Series Details == Series: New engine discovery and execbuffer2 engine selection uAPI URL : https://patchwork.freedesktop.org/series/23189/ State : failure == Summary == Series 23189v1 New engine discovery and execbuffer2 engine selection uAPI https://patchwork.freedesktop.org/api/1.0/series/23189/revisions/1/mbox/ Test drv_module_reload: Subgroup basic-reload: pass -> FAIL (fi-ilk-650) Subgroup basic-reload-final: pass -> FAIL (fi-ilk-650) Test gem_cpu_reloc: Subgroup basic: pass -> FAIL (fi-hsw-4770) pass -> FAIL (fi-bxt-t5700) pass -> FAIL (fi-kbl-7500u) pass -> FAIL (fi-skl-6700hq) pass -> FAIL (fi-skl-gvtdvm) pass -> FAIL (fi-byt-n2820) pass -> FAIL (fi-bsw-n3050) pass -> FAIL (fi-hsw-4770r) pass -> FAIL (fi-bxt-j4205) pass -> FAIL (fi-byt-j1900) pass -> FAIL (fi-skl-6770hq) pass -> FAIL (fi-ivb-3520m) pass -> FAIL (fi-bdw-gvtdvm) pass -> FAIL (fi-snb-2600) pass -> FAIL (fi-skl-6260u) pass -> FAIL (fi-skl-6700k) pass -> FAIL (fi-ivb-3770) pass -> FAIL (fi-kbl-7560u) pass -> FAIL (fi-bdw-5557u) pass -> FAIL (fi-snb-2520m) Test gem_ctx_switch: Subgroup basic-default: pass -> SKIP (fi-hsw-4770) pass -> SKIP (fi-bxt-t5700) pass -> SKIP (fi-kbl-7500u) pass -> SKIP (fi-skl-6700hq) pass -> SKIP (fi-skl-gvtdvm) pass -> SKIP (fi-byt-n2820) pass -> SKIP (fi-bsw-n3050) pass -> SKIP (fi-hsw-4770r) pass -> SKIP (fi-bxt-j4205) pass -> SKIP (fi-byt-j1900) pass -> SKIP (fi-skl-6770hq) pass -> SKIP (fi-ivb-3520m) pass -> SKIP (fi-bdw-gvtdvm) pass -> SKIP (fi-snb-2600) pass -> SKIP (fi-skl-6260u) pass -> SKIP (fi-skl-6700k) pass -> SKIP (fi-ivb-3770) pass -> SKIP (fi-kbl-7560u) pass -> SKIP (fi-bdw-5557u) pass -> SKIP (fi-snb-2520m) Subgroup basic-default-heavy: pass -> SKIP (fi-hsw-4770) pass -> SKIP (fi-bxt-t5700) pass -> SKIP (fi-kbl-7500u) pass -> SKIP (fi-skl-6700hq) pass -> SKIP (fi-skl-gvtdvm) pass -> SKIP (fi-byt-n2820) pass -> SKIP (fi-bsw-n3050) pass -> SKIP (fi-hsw-4770r) pass -> SKIP (fi-bxt-j4205) pass -> SKIP (fi-byt-j1900) pass -> SKIP (fi-skl-6770hq) pass -> SKIP (fi-ivb-3520m) pass -> SKIP (fi-bdw-gvtdvm) pass -> SKIP (fi-snb-2600) pass -> SKIP (fi-skl-6260u) pass -> SKIP (fi-skl-6700k) pass -> SKIP (fi-ivb-3770) pass -> SKIP (fi-kbl-7560u) pass -> SKIP (fi-bdw-5557u) pass -> SKIP (fi-snb-2520m) Test gem_exec_basic: Subgroup basic-blt: pass -> SKIP (fi-hsw-4770) pass -> SKIP (fi-bxt-t5700) pass -> SKIP (fi-kbl-7500u) pass -> SKIP (fi-skl-6700hq) pass -> SKIP (fi-skl-gvtdvm) pass -> SKIP (fi-byt-n2820) pass -> SKIP (fi-bsw-n3050) pass -> SKIP (fi-hsw-4770r) pass -> SKIP (fi-bxt-j4205) pass -> SKIP (fi-byt-j1900) pass -> SKIP (fi-skl-6770hq) pass -> SKIP (fi-ivb-3520m) pass -> SKIP (fi-bdw-gvtdvm) pass -> SKIP (fi-snb-2600) pass -> SKIP (fi-skl-6260u) pass -> SKIP (fi-skl-6700k) pass ->
Re: [Intel-gfx] [RFC 1/2] drm/i915: Engine discovery uAPI
On Tue, Apr 18, 2017 at 05:56:14PM +0100, Tvrtko Ursulin wrote:
> +enum drm_i915_gem_engine_class {
> + DRM_I915_ENGINE_CLASS_OTHER = 0,
> + DRM_I915_ENGINE_CLASS_RENDER = 1,
> + DRM_I915_ENGINE_CLASS_COPY = 2,
> + DRM_I915_ENGINE_CLASS_VIDEO_DECODE = 3,
> + DRM_I915_ENGINE_CLASS_VIDEO_ENHANCE = 4,
> + DRM_I915_ENGINE_CLASS_MAX /* non-ABI */
> +};
> +
> +struct drm_i915_engine_info {
> + /** Engine instance number. */
> + __u32 instance;
> + __u32 rsvd;
> +
> + /** Engine specific info. */
> +#define DRM_I915_ENGINE_HAS_HEVC BIT(0)
> + __u64 info;
> +};
So the main question is how can we extend this in future, keeping
forwards/backwards compat?
I think if we put a query version into info and then the kernel supplies
an array matching that version (or reports the most recent version
supported if the request is too modern.)
The kernel has to keep all the old struct variants and exporters
indefinitely.
Another alternative would get an ENGINE_GETPARAM where we just have a
switch of all possibily questions. Maybe better as a CONTEXT_GETPARAM if
we start thinking about allowing CONTEXT_SETPARAM to fine tune
individual clients.
> +struct drm_i915_gem_engine_info {
> + /** in: Engine class to probe (enum drm_i915_gem_engine_class). */
> + __u32 engine_class;
__u32 [in/out] version ? (see above)
> +
> + /** out: Actual number of hardware engines. */
> + __u32 num_engines;
> +
> + /**
> + * in: Number of struct drm_i915_engine_ifo entries in the provided
> + * info array.
> + */
> + __u32 info_size;
This is superfluous with num_engines. The standard 2 pass, discovery of
size, followed by allocation and final query.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 04/20] drm/i915/tdr: Modify error handler for per engine hang recovery
From: Arun Siluvery
This is a preparatory patch which modifies error handler to do per engine
hang recovery. The actual patch which implements this sequence follows
later in the series. The aim is to prepare existing recovery function to
adapt to this new function where applicable (which fails at this point
because core implementation is lacking) and continue recovery using legacy
full gpu reset.
A helper function is also added to query the availability of engine
reset.
The error events behaviour that are used to notify user of reset are
adapted to engine reset such that it doesn't break users listening to these
events. In legacy we report an error event, a reset event before resetting
the gpu and a reset done event marking the completion of reset. The same
behaviour is adapted but reset event is only dispatched once even when
multiple engines are hung. Finally once reset is complete we send reset
done event as usual.
Note that this implementation of engine reset is for i915 directly
submitting to the ELSP, where the driver manages the hang detection,
recovery and resubmission. With GuC submission these tasks are shared
between driver and firmware; i915 will still responsible for detecting a
hang, and when it does it will have to request GuC to reset that Engine and
remind the firmware about the outstanding submissions. This will be
added in different patch.
v2: rebase, advertise engine reset availability in platform definition,
add note about GuC submission.
v3: s/*engine_reset*/*reset_engine*/. (Chris)
Handle reset as 2 level resets, by first going to engine only and fall
backing to full/chip reset as needed, i.e. reset_engine will need the
struct_mutex.
v4: Pass the engine mask to i915_reset. (Chris)
v5: Rebase, update selftests.
v6: Rebase, prepare for mutex-less reset engine.
Cc: Chris Wilson
Cc: Mika Kuoppala
Signed-off-by: Ian Lister
Signed-off-by: Tomas Elf
Signed-off-by: Arun Siluvery
Signed-off-by: Michel Thierry
---
drivers/gpu/drm/i915/i915_drv.c | 15 +++
drivers/gpu/drm/i915/i915_drv.h | 6 ++
drivers/gpu/drm/i915/i915_irq.c | 30 +-
drivers/gpu/drm/i915/i915_pci.c | 5 -
drivers/gpu/drm/i915/intel_ringbuffer.h | 16
drivers/gpu/drm/i915/intel_uncore.c | 11 +++
6 files changed, 81 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index cc7393e65e99..e03d0643dbd6 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1800,6 +1800,8 @@ void i915_reset(struct drm_i915_private *dev_priv)
if (!test_bit(I915_RESET_HANDOFF, &error->flags))
return;
+ DRM_DEBUG_DRIVER("resetting chip\n");
+
/* Clear any previous failed attempts at recovery. Time to try again. */
if (!i915_gem_unset_wedged(dev_priv))
goto wakeup;
@@ -1863,6 +1865,19 @@ void i915_reset(struct drm_i915_private *dev_priv)
goto finish;
}
+/**
+ * i915_reset_engine - reset GPU engine to recover from a hang
+ * @engine: engine to reset
+ *
+ * Reset a specific GPU engine. Useful if a hang is detected.
+ * Returns zero on successful reset or otherwise an error code.
+ */
+int i915_reset_engine(struct intel_engine_cs *engine)
+{
+ /* FIXME: replace me with engine reset sequence */
+ return -ENODEV;
+}
+
static int i915_pm_suspend(struct device *kdev)
{
struct pci_dev *pdev = to_pci_dev(kdev);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e06af46f5a57..7bc5f552add7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -814,6 +814,7 @@ struct intel_csr {
func(has_ddi); \
func(has_decoupled_mmio); \
func(has_dp_mst); \
+ func(has_reset_engine); \
func(has_fbc); \
func(has_fpga_dbg); \
func(has_full_ppgtt); \
@@ -1616,6 +1617,9 @@ struct i915_gpu_error {
#define I915_RESET_HANDOFF 1
#define I915_WEDGED(BITS_PER_LONG - 1)
+ /* if available, engine-specific reset is tried before full gpu reset */
+ u32 reset_engine_mask;
+
/**
* Waitqueue to signal when a hang is detected. Used to for waiters
* to release the struct_mutex for the reset to procede.
@@ -3019,6 +3023,8 @@ extern void i915_driver_unload(struct drm_device *dev);
extern int intel_gpu_reset(struct drm_i915_private *dev_priv, u32 engine_mask);
extern bool intel_has_gpu_reset(struct drm_i915_private *dev_priv);
extern void i915_reset(struct drm_i915_private *dev_priv);
+extern int i915_reset_engine(struct intel_engine_cs *engine);
+extern bool intel_has_reset_engine(struct drm_i915_private *dev_priv);
extern int intel_guc_reset(struct drm_i915_private *dev_priv);
extern void intel_engine_init_hangcheck(struct intel_engine_cs *engine);
extern void intel_hangcheck_init(struct drm_i915_private *dev_p
[Intel-gfx] [PATCH v6 01/20] drm/i915: Fix stale comment about I915_RESET_IN_PROGRESS flag
It has been replaced by I915_RESET_BACKOFF / I915_RESET_HANDOFF.
Signed-off-by: Michel Thierry
---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b5c81de102e8..e06af46f5a57 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1568,7 +1568,7 @@ struct i915_gpu_error {
*
* This is a counter which gets incremented when reset is triggered,
*
-* Before the reset commences, the I915_RESET_IN_PROGRESS bit is set
+* Before the reset commences, the I915_RESET_BACKOFF bit is set
* meaning that any waiters holding onto the struct_mutex should
* relinquish the lock immediately in order for the reset to start.
*
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 08/20] drm/i915/tdr: Export per-engine reset count info to debugfs
From: Arun Siluvery
A new variable is added to export the reset counts to debugfs, this
includes full gpu reset and engine reset count. This is useful for tests
where they are expected to trigger reset; these counts are checked before
and after the test to ensure the same.
v2: Include reset engine count in i915_engine_info too (Chris).
Cc: Chris Wilson
Cc: Mika Kuoppala
Signed-off-by: Arun Siluvery
Signed-off-by: Michel Thierry
---
drivers/gpu/drm/i915/i915_debugfs.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 870c470177b5..6444c1a9bd22 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1403,6 +1403,23 @@ static int i915_hangcheck_info(struct seq_file *m, void
*unused)
return 0;
}
+static int i915_reset_info(struct seq_file *m, void *unused)
+{
+ struct drm_i915_private *dev_priv = node_to_i915(m->private);
+ struct i915_gpu_error *error = &dev_priv->gpu_error;
+ struct intel_engine_cs *engine;
+ enum intel_engine_id id;
+
+ seq_printf(m, "full gpu reset = %u\n", i915_reset_count(error));
+
+ for_each_engine(engine, dev_priv, id) {
+ seq_printf(m, "%s = %u\n", engine->name,
+ i915_reset_engine_count(error, engine));
+ }
+
+ return 0;
+}
+
static int ironlake_drpc_info(struct seq_file *m)
{
struct drm_i915_private *dev_priv = node_to_i915(m->private);
@@ -3242,6 +3259,7 @@ static int i915_display_info(struct seq_file *m, void
*unused)
static int i915_engine_info(struct seq_file *m, void *unused)
{
struct drm_i915_private *dev_priv = node_to_i915(m->private);
+ struct i915_gpu_error *error = &dev_priv->gpu_error;
struct intel_engine_cs *engine;
enum intel_engine_id id;
@@ -3265,6 +3283,8 @@ static int i915_engine_info(struct seq_file *m, void
*unused)
engine->hangcheck.seqno,
jiffies_to_msecs(jiffies -
engine->hangcheck.action_timestamp),
engine->timeline->inflight_seqnos);
+ seq_printf(m, "\tReset count: %d\n",
+ i915_reset_engine_count(error, engine));
rcu_read_lock();
@@ -4777,6 +4797,7 @@ static const struct drm_info_list i915_debugfs_list[] = {
{"i915_huc_load_status", i915_huc_load_status_info, 0},
{"i915_frequency_info", i915_frequency_info, 0},
{"i915_hangcheck_info", i915_hangcheck_info, 0},
+ {"i915_reset_info", i915_reset_info, 0},
{"i915_drpc_info", i915_drpc_info, 0},
{"i915_emon_status", i915_emon_status, 0},
{"i915_ring_freq_table", i915_ring_freq_table, 0},
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 07/20] drm/i915/tdr: Add engine reset count to error state
From: Arun Siluvery
Driver maintains count of how many times a given engine is reset, useful to
capture this in error state also. It gives an idea of how engine is coping
up with the workloads it is executing before this error state.
A follow-up patch will provide this information in debugfs.
v2: s/engine_reset/reset_engine/ (Chris)
Define count as unsigned int (Tvrtko)
Cc: Chris Wilson
Cc: Mika Kuoppala
Signed-off-by: Arun Siluvery
Signed-off-by: Michel Thierry
---
drivers/gpu/drm/i915/i915_drv.c | 2 ++
drivers/gpu/drm/i915/i915_drv.h | 8
drivers/gpu/drm/i915/i915_gpu_error.c | 3 +++
3 files changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 634893cd93b3..974be1fa77f9 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1935,6 +1935,8 @@ int i915_reset_engine(struct intel_engine_cs *engine)
if (ret)
goto error;
+ error->reset_engine_count[engine->id]++;
+
wakeup:
enable_irq(dev_priv->drm.irq);
intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index eb3e5bcda478..71c34f15be64 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -982,6 +982,7 @@ struct i915_gpu_state {
enum intel_engine_hangcheck_action hangcheck_action;
struct i915_address_space *vm;
int num_requests;
+ u32 reset_count;
/* position of active request inside the ring */
u32 rq_head, rq_post, rq_tail;
@@ -1619,6 +1620,7 @@ struct i915_gpu_error {
/* if available, engine-specific reset is tried before full gpu reset */
u32 reset_engine_mask;
+ u32 reset_engine_count[I915_NUM_ENGINES];
/**
* Waitqueue to signal when a hang is detected. Used to for waiters
@@ -3442,6 +3444,12 @@ static inline u32 i915_reset_count(struct i915_gpu_error
*error)
return READ_ONCE(error->reset_count);
}
+static inline u32 i915_reset_engine_count(struct i915_gpu_error *error,
+ struct intel_engine_cs *engine)
+{
+ return READ_ONCE(error->reset_engine_count[engine->id]);
+}
+
int i915_gem_reset_prepare_engine(struct intel_engine_cs *engine);
int i915_gem_reset_prepare(struct drm_i915_private *dev_priv,
unsigned int engine_mask);
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index 14e2064b7653..a2ffb1ef2cfa 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -463,6 +463,7 @@ static void error_print_engine(struct
drm_i915_error_state_buf *m,
err_printf(m, " hangcheck action timestamp: %lu, %u ms ago\n",
ee->hangcheck_timestamp,
jiffies_to_msecs(jiffies - ee->hangcheck_timestamp));
+ err_printf(m, " engine reset count: %u\n", ee->reset_count);
error_print_request(m, " ELSP[0]: ", &ee->execlist[0]);
error_print_request(m, " ELSP[1]: ", &ee->execlist[1]);
@@ -1244,6 +1245,8 @@ static void error_record_engine_registers(struct
i915_gpu_state *error,
ee->hangcheck_timestamp = engine->hangcheck.action_timestamp;
ee->hangcheck_action = engine->hangcheck.action;
ee->hangcheck_stalled = engine->hangcheck.stalled;
+ ee->reset_count = i915_reset_engine_count(&dev_priv->gpu_error,
+ engine);
if (USES_PPGTT(dev_priv)) {
int i;
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 03/20] drm/i915: Update i915.reset to handle engine resets
From: Arun Siluvery
In preparation for engine reset work update this parameter to handle more
than one type of reset. Default at the moment is still full gpu reset.
Cc: Chris Wilson
Cc: Mika Kuoppala
Signed-off-by: Arun Siluvery
Signed-off-by: Michel Thierry
---
drivers/gpu/drm/i915/i915_params.c | 6 +++---
drivers/gpu/drm/i915/i915_params.h | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_params.c
b/drivers/gpu/drm/i915/i915_params.c
index b6a7e363d076..045cadb77285 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -46,7 +46,7 @@ struct i915_params i915 __read_mostly = {
.prefault_disable = 0,
.load_detect_test = 0,
.force_reset_modeset_test = 0,
- .reset = true,
+ .reset = 1,
.error_capture = true,
.invert_brightness = 0,
.disable_display = 0,
@@ -115,8 +115,8 @@ MODULE_PARM_DESC(vbt_sdvo_panel_type,
"Override/Ignore selection of SDVO panel mode in the VBT "
"(-2=ignore, -1=auto [default], index in VBT BIOS table)");
-module_param_named_unsafe(reset, i915.reset, bool, 0600);
-MODULE_PARM_DESC(reset, "Attempt GPU resets (default: true)");
+module_param_named_unsafe(reset, i915.reset, int, 0600);
+MODULE_PARM_DESC(reset, "Attempt GPU resets (0=disabled, 1=full gpu reset
[default], 2=engine reset)");
#if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR)
module_param_named(error_capture, i915.error_capture, bool, 0600);
diff --git a/drivers/gpu/drm/i915/i915_params.h
b/drivers/gpu/drm/i915/i915_params.h
index 34148cc8637c..febbfdbd30bd 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -51,6 +51,7 @@
func(int, use_mmio_flip); \
func(int, mmio_debug); \
func(int, edp_vswing); \
+ func(int, reset); \
func(unsigned int, inject_load_failure); \
/* leave bools at the end to not create holes */ \
func(bool, alpha_support); \
@@ -60,7 +61,6 @@
func(bool, prefault_disable); \
func(bool, load_detect_test); \
func(bool, force_reset_modeset_test); \
- func(bool, reset); \
func(bool, error_capture); \
func(bool, disable_display); \
func(bool, verbose_state_checks); \
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 02/20] drm/i915: Rename gen8_(un)request_engine_reset to gen8_reset_engine_start/cancel
As all other functions related to resetting engines are using
reset_engine.
v2: remove _request_ and use start/cancel instead (Chris)
Cc: Chris Wilson
Signed-off-by: Michel Thierry
---
drivers/gpu/drm/i915/intel_uncore.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index a10d8863b0a9..07a722f74fa1 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1683,7 +1683,7 @@ int intel_wait_for_register(struct drm_i915_private
*dev_priv,
return ret;
}
-static int gen8_request_engine_reset(struct intel_engine_cs *engine)
+static int gen8_reset_engine_start(struct intel_engine_cs *engine)
{
struct drm_i915_private *dev_priv = engine->i915;
int ret;
@@ -1702,7 +1702,7 @@ static int gen8_request_engine_reset(struct
intel_engine_cs *engine)
return ret;
}
-static void gen8_unrequest_engine_reset(struct intel_engine_cs *engine)
+static void gen8_reset_engine_cancel(struct intel_engine_cs *engine)
{
struct drm_i915_private *dev_priv = engine->i915;
@@ -1717,14 +1717,14 @@ static int gen8_reset_engines(struct drm_i915_private
*dev_priv,
unsigned int tmp;
for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
- if (gen8_request_engine_reset(engine))
+ if (gen8_reset_engine_start(engine))
goto not_ready;
return gen6_reset_engines(dev_priv, engine_mask);
not_ready:
for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
- gen8_unrequest_engine_reset(engine);
+ gen8_reset_engine_cancel(engine);
return -EIO;
}
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 06/20] drm/i915: Skip reset request if there is one already
From: Mika Kuoppala
To perform engine reset we first disable engine to capture its state. This
is done by issuing a reset request. Because we are reusing existing
infrastructure, again when we actually reset an engine, reset function
checks engine mask and issues reset request again which is unnecessary. To
avoid this we check if the engine is already prepared, if so we just exit
from that point.
Cc: Chris Wilson
Signed-off-by: Mika Kuoppala
Signed-off-by: Arun Siluvery
Signed-off-by: Michel Thierry
---
drivers/gpu/drm/i915/intel_uncore.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 3ebba6b2dd74..120fb440bb8b 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1686,10 +1686,15 @@ int intel_wait_for_register(struct drm_i915_private
*dev_priv,
static int gen8_reset_engine_start(struct intel_engine_cs *engine)
{
struct drm_i915_private *dev_priv = engine->i915;
+ const i915_reg_t reset_ctrl = RING_RESET_CTL(engine->mmio_base);
+ const u32 ready = RESET_CTL_REQUEST_RESET | RESET_CTL_READY_TO_RESET;
int ret;
- I915_WRITE_FW(RING_RESET_CTL(engine->mmio_base),
- _MASKED_BIT_ENABLE(RESET_CTL_REQUEST_RESET));
+ /* If engine has been already prepared, we can shortcut here */
+ if ((I915_READ_FW(reset_ctrl) & ready) == ready)
+ return 0;
+
+ I915_WRITE_FW(reset_ctrl, _MASKED_BIT_ENABLE(RESET_CTL_REQUEST_RESET));
ret = intel_wait_for_register_fw(dev_priv,
RING_RESET_CTL(engine->mmio_base),
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 05/20] drm/i915/tdr: Add support for per engine reset recovery
From: Arun Siluvery
This change implements support for per-engine reset as an initial, less
intrusive hang recovery option to be attempted before falling back to the
legacy full GPU reset recovery mode if necessary. This is only supported
from Gen8 onwards.
Hangchecker determines which engines are hung and invokes error handler to
recover from it. Error handler schedules recovery for each of those engines
that are hung. The recovery procedure is as follows,
- identifies the request that caused the hang and it is dropped
- force engine to idle: this is done by issuing a reset request
- reset and re-init engine
- restart submissions to the engine
If engine reset fails then we fall back to heavy weight full gpu reset
which resets all engines and reinitiazes complete state of HW and SW.
v2: Rebase.
v3: s/*engine_reset*/*reset_engine*/; freeze engine and irqs before
calling i915_gem_reset_engine (Chris).
v4: Rebase, modify i915_gem_reset_prepare to use a ring mask and
reuse the function for reset_engine.
v5: intel_reset_engine_start/cancel instead of request/unrequest_reset.
v6: Clean up reset_engine function to not require mutex, i.e. no need to call
revoke/restore_fences and _retire_requests (Chris).
Cc: Chris Wilson
Cc: Mika Kuoppala
Signed-off-by: Tomas Elf
Signed-off-by: Arun Siluvery
Signed-off-by: Michel Thierry
---
drivers/gpu/drm/i915/i915_drv.c | 76 --
drivers/gpu/drm/i915/i915_drv.h | 12 +++-
drivers/gpu/drm/i915/i915_gem.c | 97 +++--
drivers/gpu/drm/i915/i915_gem_request.c | 2 +-
drivers/gpu/drm/i915/intel_uncore.c | 20 +++
5 files changed, 158 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e03d0643dbd6..634893cd93b3 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1810,7 +1810,7 @@ void i915_reset(struct drm_i915_private *dev_priv)
pr_notice("drm/i915: Resetting chip after gpu hang\n");
disable_irq(dev_priv->drm.irq);
- ret = i915_gem_reset_prepare(dev_priv);
+ ret = i915_gem_reset_prepare(dev_priv, ALL_ENGINES);
if (ret) {
DRM_ERROR("GPU recovery failed\n");
intel_gpu_reset(dev_priv, ALL_ENGINES);
@@ -1852,7 +1852,7 @@ void i915_reset(struct drm_i915_private *dev_priv)
i915_queue_hangcheck(dev_priv);
finish:
- i915_gem_reset_finish(dev_priv);
+ i915_gem_reset_finish(dev_priv, ALL_ENGINES);
enable_irq(dev_priv->drm.irq);
wakeup:
@@ -1871,11 +1871,79 @@ void i915_reset(struct drm_i915_private *dev_priv)
*
* Reset a specific GPU engine. Useful if a hang is detected.
* Returns zero on successful reset or otherwise an error code.
+ *
+ * Procedure is:
+ * - identifies the request that caused the hang and it is dropped
+ * - force engine to idle: this is done by issuing a reset request
+ * - reset engine
+ * - restart submissions to the engine
*/
int i915_reset_engine(struct intel_engine_cs *engine)
{
- /* FIXME: replace me with engine reset sequence */
- return -ENODEV;
+ int ret;
+ struct drm_i915_private *dev_priv = engine->i915;
+ struct i915_gpu_error *error = &dev_priv->gpu_error;
+
+ GEM_BUG_ON(!test_bit(I915_RESET_BACKOFF, &error->flags));
+
+ DRM_DEBUG_DRIVER("resetting %s\n", engine->name);
+
+ /*
+* We need to first idle the engine by issuing a reset request,
+* then perform soft reset and re-initialize hw state, for all of
+* this GT power need to be awake so ensure it does throughout the
+* process
+*/
+ intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
+
+ disable_irq(dev_priv->drm.irq);
+ ret = i915_gem_reset_prepare_engine(engine);
+ if (ret) {
+ DRM_ERROR("Previous reset failed - promote to full reset\n");
+ goto error;
+ }
+
+ /*
+* the request that caused the hang is stuck on elsp, identify the
+* active request and drop it, adjust head to skip the offending
+* request to resume executing remaining requests in the queue.
+*/
+ i915_gem_reset_engine(engine);
+
+ /* forcing engine to idle */
+ ret = intel_reset_engine_start(engine);
+ if (ret) {
+ DRM_ERROR("Failed to disable %s\n", engine->name);
+ goto error;
+ }
+
+ /* finally, reset engine */
+ ret = intel_gpu_reset(dev_priv, intel_engine_flag(engine));
+ if (ret) {
+ DRM_ERROR("Failed to reset %s, ret=%d\n", engine->name, ret);
+ intel_reset_engine_cancel(engine);
+ goto error;
+ }
+
+ /* be sure the request reset bit gets cleared */
+ intel_reset_engine_cancel(engine);
+
+ i915_gem_reset_finish_engine(engine);
+
+ /* replay remaining requests in the queue */
+ ret = engi
[Intel-gfx] [PATCH v6 00/20] Gen8+ engine-reset
These patches add the reset-engine feature from Gen8. This is also referred to as Timeout detection and recovery (TDR). This complements to the full gpu reset feature available in i915 but it only allows to reset a particular engine instead of all engines thus providing a light weight engine reset and recovery mechanism. Thanks to recent changes merged, this implementation is now not only for execlists, but for GuC based submission too; it is still limited from Gen8 onwards. I have also included the changes for watchdog timeout detection. The GuC related patches are functional, but can be seen as RFC. Timeout detection relies on the existing hangcheck, which remains the same; main changes are to the recovery mechanism. Once we detect a hang on a particular engine we identify the request that caused the hang, skip the request and adjust head pointers to allow the execution to proceed normally. After some cleanup, submissions are restarted to process remaining work queued to that engine. If engine reset fails to recover engine correctly then we fallback to full gpu reset. We can argue about the effectiveness of reset-engine vs full reset when more than one ring is hung, but the benefits of just resetting one engine are reduced when the driver has to do it multiple times. v2: ELSP queue request tracking and reset path changes to handle incomplete requests during reset. Thanks to Chris Wilson for providing these patches. v3: Let the waiter keep handling the full gpu reset if it already has the lock; point out that GuC submission needs a different method to restart workloads after the engine reset completes. v4: Handle reset as 2 level resets, by first going to engine only and fall backing to full/chip reset as needed, i.e. reset_engine will need the struct_mutex. v5: Rebased after reset flag split in 2, add GuC support, include watchdog detection patches, addressing comments from prev RFC. v6: Mutex-less reset engine. Updates in watchdog abi and guc whitelist & register-restore fixes (including an old patch from Daniele). Cc: Chris Wilson Cc: Mika Kuoppala Cc: Daniele Ceraolo Spurio Arun Siluvery (7): drm/i915: Update i915.reset to handle engine resets drm/i915/tdr: Modify error handler for per engine hang recovery drm/i915/tdr: Add support for per engine reset recovery drm/i915/tdr: Add engine reset count to error state drm/i915/tdr: Export per-engine reset count info to debugfs drm/i915/tdr: Enable Engine reset and recovery support drm/i915/guc: Provide register list to be saved/restored during engine reset Daniele Ceraolo Spurio (1): drm/i915/guc: fix mmio whitelist mmio_start offset and add reminder Michel Thierry (11): drm/i915: Fix stale comment about I915_RESET_IN_PROGRESS flag drm/i915: Rename gen8_(un)request_engine_reset to gen8_reset_engine_start/cancel drm/i915: Add engine reset count in get-reset-stats ioctl drm/i915/selftests: reset engine self tests drm/i915/guc: Add support for reset engine using GuC commands drm/i915: Watchdog timeout: Pass GuC shared data structure during param load drm/i915: Watchdog timeout: IRQ handler for gen8+ drm/i915: Watchdog timeout: Ringbuffer command emission for gen8+ drm/i915: Watchdog timeout: DRM kernel interface to set the timeout drm/i915: Watchdog timeout: Include threshold value in error state drm/i915: Watchdog timeout: Export media reset count from GuC to debugfs Mika Kuoppala (1): drm/i915: Skip reset request if there is one already drivers/gpu/drm/i915/i915_debugfs.c | 43 +++ drivers/gpu/drm/i915/i915_drv.c | 104 ++- drivers/gpu/drm/i915/i915_drv.h | 63 - drivers/gpu/drm/i915/i915_gem.c | 97 -- drivers/gpu/drm/i915/i915_gem_context.c | 116 - drivers/gpu/drm/i915/i915_gem_context.h | 4 + drivers/gpu/drm/i915/i915_gem_request.c | 2 +- drivers/gpu/drm/i915/i915_gpu_error.c| 14 +- drivers/gpu/drm/i915/i915_guc_submission.c | 128 +-- drivers/gpu/drm/i915/i915_irq.c | 43 ++- drivers/gpu/drm/i915/i915_params.c | 6 +- drivers/gpu/drm/i915/i915_params.h | 2 +- drivers/gpu/drm/i915/i915_pci.c | 5 +- drivers/gpu/drm/i915/i915_reg.h | 6 + drivers/gpu/drm/i915/intel_guc_fwif.h| 27 +++- drivers/gpu/drm/i915/intel_guc_loader.c | 11 ++ drivers/gpu/drm/i915/intel_hangcheck.c | 13 +- drivers/gpu/drm/i915/intel_lrc.c | 156 ++- drivers/gpu/drm/i915/intel_ringbuffer.h | 24 drivers/gpu/drm/i915/intel_uc.h | 3 + drivers/gpu/drm/i915/intel_uncore.c | 43 ++- drivers/gpu/drm/i915/selftests/intel_hangcheck.c | 147 + include/uapi/drm/i915_drm.h | 7 +-
[Intel-gfx] [PATCH v6 18/20] drm/i915: Watchdog timeout: DRM kernel interface to set the timeout
Final enablement patch for GPU hang detection using watchdog timeout.
Using the gem_context_setparam ioctl, users can specify the desired
timeout value in microseconds, and the driver will do the conversion to
'timestamps'.
The recommended default watchdog threshold for video engines is 6 us,
since this has been _empirically determined_ to be a good compromise for
low-latency requirements and low rate of false positives. The default
register value is ~106000us and the theoretical max value (all 1s) is
353 seconds.
Note, UABI engine ids and i915 engine ids are different, and this patch
uses the i915 ones. Some kind of mapping table [1] is required if we
decide to use the UABI engine ids.
[1]
http://patchwork.freedesktop.org/patch/msgid/20170329135831.30254-2-ch...@chris-wilson.co.uk
v2: Fixed get api to return values in microseconds. Threshold updated to
be per context engine. Check for u32 overflow. Capture ctx threshold
value in error state.
v3: Add a way to get array size, short-cut to disable all thresholds,
return EFAULT / EINVAL as needed. Move the capture of the threshold
value in the error state into a new patch. BXT has a different
timestamp base (because why not?).
Signed-off-by: Tomas Elf
Signed-off-by: Arun Siluvery
Signed-off-by: Michel Thierry
---
drivers/gpu/drm/i915/i915_drv.h | 29 +
drivers/gpu/drm/i915/i915_gem_context.c | 102
drivers/gpu/drm/i915/intel_lrc.c| 5 +-
include/uapi/drm/i915_drm.h | 1 +
4 files changed, 135 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 203f2112dd18..f65a236fddef 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3574,6 +3574,35 @@ i915_gem_context_lookup_timeline(struct i915_gem_context
*ctx,
return &vm->timeline.engine[engine->id];
}
+/*
+ * BDW & SKL+ Timestamp timer resolution = 0.080 uSec,
+ * or 1250 counts per second, or ~12 counts per microsecond.
+ *
+ * But Broxton Timestamp timer resolution is different, 0.052 uSec,
+ * or 1920 counts per second, or ~19 counts per microsecond.
+ */
+#define SKL_TIMESTAMP_CNTS_PER_USEC 12
+#define BXT_TIMESTAMP_CNTS_PER_USEC 19
+#define TIMESTAMP_CNTS_PER_USEC(dev_priv) (IS_BROXTON(dev_priv) ? \
+ BXT_TIMESTAMP_CNTS_PER_USEC : \
+ SKL_TIMESTAMP_CNTS_PER_USEC)
+static inline u32
+watchdog_to_us(struct drm_i915_private *dev_priv, u32 value_in_clock_counts)
+{
+ return value_in_clock_counts / TIMESTAMP_CNTS_PER_USEC(dev_priv);
+}
+
+static inline u32
+watchdog_to_clock_counts(struct drm_i915_private *dev_priv, u64 value_in_us)
+{
+ u64 threshold = value_in_us * TIMESTAMP_CNTS_PER_USEC(dev_priv);
+
+ if (overflows_type(threshold, u32))
+ return -EINVAL;
+
+ return threshold;
+}
+
int i915_perf_open_ioctl(struct drm_device *dev, void *data,
struct drm_file *file);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index edbed85a1c88..85a6467a25a6 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -422,6 +422,102 @@ i915_gem_context_create_gvt(struct drm_device *dev)
return ctx;
}
+/* Return the timer count threshold in microseconds. */
+int i915_gem_context_get_watchdog(struct i915_gem_context *ctx,
+ struct drm_i915_gem_context_param *args)
+{
+ struct drm_i915_private *dev_priv = ctx->i915;
+ struct intel_engine_cs *engine;
+ enum intel_engine_id id;
+ u32 threshold_in_us[I915_NUM_ENGINES];
+
+ if (args->size == 0)
+ goto out;
+
+ if (args->size < sizeof(threshold_in_us))
+ return -EFAULT;
+
+ if (!dev_priv->engine[VCS]->emit_start_watchdog)
+ return -ENODEV;
+
+ for_each_engine(engine, dev_priv, id) {
+ struct intel_context *ce = &ctx->engine[id];
+
+ threshold_in_us[id] = watchdog_to_us(dev_priv,
+ce->watchdog_threshold);
+ }
+
+ mutex_unlock(&dev_priv->drm.struct_mutex);
+ if (__copy_to_user(u64_to_user_ptr(args->value),
+ &threshold_in_us,
+ sizeof(threshold_in_us))) {
+ mutex_lock(&dev_priv->drm.struct_mutex);
+ return -EFAULT;
+ }
+ mutex_lock(&dev_priv->drm.struct_mutex);
+
+out:
+ args->size = sizeof(threshold_in_us);
+
+ return 0;
+}
+
+/*
+ * Based on time out value in microseconds (us) calculate
+ * timer count thresholds needed based on core frequency.
+ * Watchdog can be disabled by setting it to 0.
+ */
+int i915_gem_context_set_watchdog(struct i915_gem_context *ctx,
+ struct drm_i915_gem_context
[Intel-gfx] [PATCH v6 15/20] drm/i915: Watchdog timeout: Pass GuC shared data structure during param load
For watchdog / media reset, the firmware must know the address of the shared
data page (the first page of the default context).
This information should be in DWORD 9 of the GUC_CTL structure.
v2: Use guc_ggtt_offset (Chris).
Store the ggtt offset of the default ctx as we needed for
suspend/resume/reset (Daniele).
Cc: Chris Wilson
Cc: Daniele Ceraolo Spurio
Signed-off-by: Michel Thierry
---
drivers/gpu/drm/i915/i915_guc_submission.c | 21 ++---
drivers/gpu/drm/i915/intel_guc_fwif.h | 2 +-
drivers/gpu/drm/i915/intel_guc_loader.c| 11 +++
drivers/gpu/drm/i915/intel_uc.h| 2 ++
4 files changed, 20 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
b/drivers/gpu/drm/i915/i915_guc_submission.c
index c8067aeab6f4..58d6c570a188 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -1364,7 +1364,6 @@ void i915_guc_submission_reenable_engine(struct
intel_engine_cs *engine)
int intel_guc_suspend(struct drm_i915_private *dev_priv)
{
struct intel_guc *guc = &dev_priv->guc;
- struct i915_gem_context *ctx;
u32 data[3];
if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
@@ -1372,13 +1371,11 @@ int intel_guc_suspend(struct drm_i915_private *dev_priv)
gen9_disable_guc_interrupts(dev_priv);
- ctx = dev_priv->kernel_context;
-
data[0] = INTEL_GUC_ACTION_ENTER_S_STATE;
/* any value greater than GUC_POWER_D0 */
data[1] = GUC_POWER_D1;
- /* first page is shared data with GuC */
- data[2] = guc_ggtt_offset(ctx->engine[RCS].state);
+ /* first page of default ctx is shared data with GuC */
+ data[2] = guc->shared_data_offset;
return intel_guc_send(guc, data, ARRAY_SIZE(data));
}
@@ -1390,7 +1387,6 @@ int intel_guc_suspend(struct drm_i915_private *dev_priv)
int intel_guc_resume(struct drm_i915_private *dev_priv)
{
struct intel_guc *guc = &dev_priv->guc;
- struct i915_gem_context *ctx;
u32 data[3];
if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
@@ -1399,12 +1395,10 @@ int intel_guc_resume(struct drm_i915_private *dev_priv)
if (i915.guc_log_level >= 0)
gen9_enable_guc_interrupts(dev_priv);
- ctx = dev_priv->kernel_context;
-
data[0] = INTEL_GUC_ACTION_EXIT_S_STATE;
data[1] = GUC_POWER_D0;
- /* first page is shared data with GuC */
- data[2] = guc_ggtt_offset(ctx->engine[RCS].state);
+ /* first page of default ctx is shared data with GuC */
+ data[2] = guc->shared_data_offset;
return intel_guc_send(guc, data, ARRAY_SIZE(data));
}
@@ -1413,14 +1407,11 @@ int i915_guc_reset_engine(struct intel_engine_cs
*engine)
{
struct drm_i915_private *dev_priv = engine->i915;
struct intel_guc *guc = &dev_priv->guc;
- struct i915_gem_context *ctx;
u32 data[7];
if (!i915.enable_guc_submission)
return 0;
- ctx = dev_priv->kernel_context;
-
/*
* The affected context report is populated by GuC and is provided
* to the driver using the shared page. We request for it but don't
@@ -1432,8 +1423,8 @@ int i915_guc_reset_engine(struct intel_engine_cs *engine)
data[3] = 0;
data[4] = 0;
data[5] = guc->execbuf_client->stage_id;
- /* first page is shared data with GuC */
- data[6] = guc_ggtt_offset(ctx->engine[RCS].state);
+ /* first page of default ctx is shared data with GuC */
+ data[6] = guc->shared_data_offset;
return intel_guc_send(guc, data, ARRAY_SIZE(data));
}
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h
b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 081f2cf614e6..a2d0cba2f8b9 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -135,7 +135,7 @@
#define GUC_ADS_ADDR_SHIFT 11
#define GUC_ADS_ADDR_MASK0xf800
-#define GUC_CTL_RSRVD 9
+#define GUC_CTL_SHARED_DATA9
#define GUC_CTL_MAX_DWORDS (SOFT_SCRATCH_COUNT - 2) /* [1..14] */
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
b/drivers/gpu/drm/i915/intel_guc_loader.c
index d9045b6e897b..8cd5c2bf9510 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -108,6 +108,7 @@ static void guc_params_init(struct drm_i915_private
*dev_priv)
{
struct intel_guc *guc = &dev_priv->guc;
u32 params[GUC_CTL_MAX_DWORDS];
+ struct i915_gem_context *ctx;
int i;
memset(¶ms, 0, sizeof(params));
@@ -156,6 +157,16 @@ static void guc_params_init(struct drm_i915_private
*dev_priv)
params[GUC_CTL_FEATURE] &= ~GUC_CTL_DISABLE_SCHEDULER;
}
+ /*
+* For watchdog / media reset, GuC must know the address of the shared
+* data page, which is the
[Intel-gfx] [PATCH v6 20/20] drm/i915: Watchdog timeout: Export media reset count from GuC to debugfs
From firmware v8.8, GuC provides the count of media engine resets
(watchdog timeout). This information is available in the GuC shared
context data struct, which resides in the first page of the default
(kernel) lrc context.
Since GuC handled engine resets are transparent for kernel and user,
provide a simple debugfs entry to see the number of times media reset
has happened.
v2: Remove unnecessary struct_mutex, _get_dirty_page and kmap_atomic;
use READ_ONCE. (Chris)
Cc: Chris Wilson
Signed-off-by: Michel Thierry
---
drivers/gpu/drm/i915/i915_debugfs.c | 22 ++
drivers/gpu/drm/i915/intel_guc_fwif.h | 18 ++
2 files changed, 40 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 6444c1a9bd22..35ce771c8b8f 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1403,6 +1403,26 @@ static int i915_hangcheck_info(struct seq_file *m, void
*unused)
return 0;
}
+static u32 i915_watchdog_reset_count(struct drm_i915_private *dev_priv)
+{
+ struct i915_gem_context *ctx;
+ struct page *page;
+ struct guc_shared_ctx_data *guc_shared_data;
+ u32 guc_media_reset_count;
+
+ if (!i915.enable_guc_submission)
+ return 0;
+
+ ctx = dev_priv->kernel_context;
+ page = i915_gem_object_get_page(ctx->engine[RCS].state->obj,
+ LRC_GUCSHR_PN);
+ guc_shared_data = kmap(page);
+ guc_media_reset_count = READ_ONCE(guc_shared_data->media_reset_count);
+ kunmap(page);
+
+ return guc_media_reset_count;
+}
+
static int i915_reset_info(struct seq_file *m, void *unused)
{
struct drm_i915_private *dev_priv = node_to_i915(m->private);
@@ -1411,6 +1431,8 @@ static int i915_reset_info(struct seq_file *m, void
*unused)
enum intel_engine_id id;
seq_printf(m, "full gpu reset = %u\n", i915_reset_count(error));
+ seq_printf(m, "GuC watchdog/media reset = %u\n",
+ i915_watchdog_reset_count(dev_priv));
for_each_engine(engine, dev_priv, id) {
seq_printf(m, "%s = %u\n", engine->name,
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h
b/drivers/gpu/drm/i915/intel_guc_fwif.h
index a2d0cba2f8b9..e45987f7aa50 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -502,6 +502,24 @@ union guc_log_control {
u32 value;
} __packed;
+/* GuC Shared Context Data Struct */
+struct guc_shared_ctx_data {
+ u32 addr_of_last_preempted_data_low;
+ u32 addr_of_last_preempted_data_high;
+ u32 addr_of_last_preempted_data_high_tmp;
+ u32 padding;
+ u32 is_mapped_to_proxy;
+ u32 proxy_ctx_id;
+ u32 engine_reset_ctx_id;
+ u32 media_reset_count;
+ u32 reserved[8];
+ u32 uk_last_ctx_switch_reason;
+ u32 was_reset;
+ u32 lrca_gpu_addr;
+ u32 execlist_ctx;
+ u32 reserved1[32];
+} __packed;
+
/* This Action will be programmed in C180 - SOFT_SCRATCH_O_REG */
enum intel_guc_action {
INTEL_GUC_ACTION_DEFAULT = 0x0,
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 19/20] drm/i915: Watchdog timeout: Include threshold value in error state
Save the watchdog threshold (in us) as part of the engine state.
Signed-off-by: Michel Thierry
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gpu_error.c | 11 +++
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f65a236fddef..02ee26199cfe 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1022,6 +1022,7 @@ struct i915_gpu_state {
int ban_score;
int active;
int guilty;
+ int watchdog_threshold;
} context;
struct drm_i915_error_object {
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index a2ffb1ef2cfa..1b1a49bc0c3c 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -388,9 +388,10 @@ static void error_print_context(struct
drm_i915_error_state_buf *m,
const char *header,
const struct drm_i915_error_context *ctx)
{
- err_printf(m, "%s%s[%d] user_handle %d hw_id %d, ban score %d guilty %d
active %d\n",
+ err_printf(m, "%s%s[%d] user_handle %d hw_id %d, ban score %d guilty %d
active %d, watchdog %dus\n",
header, ctx->comm, ctx->pid, ctx->handle, ctx->hw_id,
- ctx->ban_score, ctx->guilty, ctx->active);
+ ctx->ban_score, ctx->guilty, ctx->active,
+ watchdog_to_us(m->i915, ctx->watchdog_threshold));
}
static void error_print_engine(struct drm_i915_error_state_buf *m,
@@ -1344,7 +1345,8 @@ static void error_record_engine_execlists(struct
intel_engine_cs *engine,
}
static void record_context(struct drm_i915_error_context *e,
- struct i915_gem_context *ctx)
+ struct i915_gem_context *ctx,
+ u32 engine_id)
{
if (ctx->pid) {
struct task_struct *task;
@@ -1363,6 +1365,7 @@ static void record_context(struct drm_i915_error_context
*e,
e->ban_score = ctx->ban_score;
e->guilty = ctx->guilty_count;
e->active = ctx->active_count;
+ e->watchdog_threshold = ctx->engine[engine_id].watchdog_threshold;
}
static void request_record_user_bo(struct drm_i915_gem_request *request,
@@ -1426,7 +1429,7 @@ static void i915_gem_record_rings(struct drm_i915_private
*dev_priv,
ee->vm = request->ctx->ppgtt ?
&request->ctx->ppgtt->base : &ggtt->base;
- record_context(&ee->context, request->ctx);
+ record_context(&ee->context, request->ctx, engine->id);
/* We need to copy these to an anonymous buffer
* as the simplest method to avoid being overwritten
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 17/20] drm/i915: Watchdog timeout: Ringbuffer command emission for gen8+
Emit the required commands into the ring buffer for starting and
stopping the watchdog timer before/after batch buffer start during
batch buffer submission.
v2: Support watchdog threshold per context engine, merge lri commands,
and move watchdog commands emission to emit_bb_start. Request space of
combined start_watchdog, bb_start and stop_watchdog to avoid any error
after emitting bb_start.
Signed-off-by: Tomas Elf
Signed-off-by: Ian Lister
Signed-off-by: Arun Siluvery
Signed-off-by: Michel Thierry
---
drivers/gpu/drm/i915/i915_gem_context.h | 4 ++
drivers/gpu/drm/i915/intel_lrc.c| 81 -
drivers/gpu/drm/i915/intel_ringbuffer.h | 4 ++
3 files changed, 87 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.h
b/drivers/gpu/drm/i915/i915_gem_context.h
index 4af2ab94558b..88700bdbb4e1 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/i915_gem_context.h
@@ -150,6 +150,10 @@ struct i915_gem_context {
u32 *lrc_reg_state;
u64 lrc_desc;
int pin_count;
+ /** watchdog_threshold: hw watchdog threshold value,
+* in clock counts
+*/
+ u32 watchdog_threshold;
bool initialised;
} engine[I915_NUM_ENGINES];
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 2263b9fb9b50..7a202e73ce9b 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1310,6 +1310,8 @@ static int gen8_emit_bb_start(struct drm_i915_gem_request
*req,
const unsigned int flags)
{
u32 *cs;
+ u32 num_dwords;
+ bool watchdog_running = false;
int ret;
/* Don't rely in hw updating PDPs, specially in lite-restore.
@@ -1329,10 +1331,29 @@ static int gen8_emit_bb_start(struct
drm_i915_gem_request *req,
req->ctx->ppgtt->pd_dirty_rings &=
~intel_engine_flag(req->engine);
}
- cs = intel_ring_begin(req, 4);
+ /* bb_start only */
+ num_dwords = 4;
+
+ /* check if watchdog will be required */
+ if (req->ctx->engine[req->engine->id].watchdog_threshold != 0) {
+ if (!req->engine->emit_start_watchdog ||
+ !req->engine->emit_stop_watchdog)
+ return -EINVAL;
+
+ /* + start_watchdog (6) + stop_watchdog (4) */
+ num_dwords += 10;
+ watchdog_running = true;
+ }
+
+ cs = intel_ring_begin(req, num_dwords);
if (IS_ERR(cs))
return PTR_ERR(cs);
+ if (watchdog_running) {
+ /* Start watchdog timer */
+ cs = req->engine->emit_start_watchdog(req, cs);
+ }
+
/* FIXME(BDW): Address space and security selectors. */
*cs++ = MI_BATCH_BUFFER_START_GEN8 |
(flags & I915_DISPATCH_SECURE ? 0 : BIT(8)) |
@@ -1340,8 +1361,13 @@ static int gen8_emit_bb_start(struct
drm_i915_gem_request *req,
*cs++ = lower_32_bits(offset);
*cs++ = upper_32_bits(offset);
*cs++ = MI_NOOP;
- intel_ring_advance(req, cs);
+ if (watchdog_running) {
+ /* Cancel watchdog timer */
+ cs = req->engine->emit_stop_watchdog(req, cs);
+ }
+
+ intel_ring_advance(req, cs);
return 0;
}
@@ -1508,6 +1534,49 @@ static void gen8_watchdog_irq_handler(unsigned long data)
intel_uncore_forcewake_put(dev_priv, engine->fw_domains);
}
+static u32 *gen8_emit_start_watchdog(struct drm_i915_gem_request *req, u32 *cs)
+{
+ struct intel_engine_cs *engine = req->engine;
+ struct i915_gem_context *ctx = req->ctx;
+ struct intel_context *ce = &ctx->engine[engine->id];
+
+ /* XXX: no watchdog support in BCS engine */
+ GEM_BUG_ON(engine->id == BCS);
+
+ /*
+* watchdog register must never be programmed to zero. This would
+* cause the watchdog counter to exceed and not allow the engine to
+* go into IDLE state
+*/
+ GEM_BUG_ON(ce->watchdog_threshold == 0);
+
+ /* Set counter period */
+ *cs++ = MI_LOAD_REGISTER_IMM(2);
+ *cs++ = i915_mmio_reg_offset(RING_THRESH(engine->mmio_base));
+ *cs++ = ce->watchdog_threshold;
+ /* Start counter */
+ *cs++ = i915_mmio_reg_offset(RING_CNTR(engine->mmio_base));
+ *cs++ = GEN8_WATCHDOG_ENABLE;
+ *cs++ = MI_NOOP;
+
+ return cs;
+}
+
+static u32 *gen8_emit_stop_watchdog(struct drm_i915_gem_request *req, u32 *cs)
+{
+ struct intel_engine_cs *engine = req->engine;
+
+ /* XXX: no watchdog support in BCS engine */
+ GEM_BUG_ON(engine->id == BCS);
+
+ *cs++ = MI_LOAD_REGISTER_IMM(2);
+ *cs++ = i915_mmio_reg_offset(RING_CNTR(engine->mmio_base));
+ *cs++ = get_watchdog_disable(engine);
+ *cs++ = MI_NOOP;
+
+ return cs;
+}
[Intel-gfx] [PATCH v6 13/20] drm/i915/guc: Provide register list to be saved/restored during engine reset
From: Arun Siluvery
GuC expects a list of registers from the driver which are saved/restored
during engine reset. The type of value to be saved is controlled by
flags. We provide a minimal set of registers that we want GuC to save and
restore. This is not an issue in case of engine reset as driver initializes
most of them following an engine reset, but in case of media reset (aka
watchdog reset) which is completely internal to GuC (including resubmission
of hung workload), it is necessary to provide this list, otherwise GuC won't
be able to schedule further workloads after a reset. This is the minimal
set of registers identified for things to work as expected but if we see
any new issues, this register list can be expanded.
v2: REGSET_MASKED is too difficult for GuC, use REGSET_SAVE_DEFAULT_VALUE
and current value from RING_MODE reg instead; no need to preserve
head/tail either, be extra paranoid and save whitelisted registers (Daniele).
Cc: Daniele Ceraolo Spurio
Signed-off-by: Arun Siluvery
Signed-off-by: Jeff McGee
Signed-off-by: Michel Thierry
---
drivers/gpu/drm/i915/i915_guc_submission.c | 60 +-
1 file changed, 58 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
b/drivers/gpu/drm/i915/i915_guc_submission.c
index 1ea36a88d2fb..d772718861df 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -1003,6 +1003,24 @@ static void guc_policies_init(struct guc_policies
*policies)
policies->is_valid = 1;
}
+/*
+ * In this macro it is highly unlikely to exceed max value but even if we did
+ * it is not an error so just throw a warning and continue. Only side effect
+ * in continuing further means some registers won't be added to save/restore
+ * list.
+ */
+#define GUC_ADD_MMIO_REG_ADS(node, reg_addr, _flags, defvalue) \
+ do {\
+ u32 __count = node->number_of_registers;\
+ if (WARN_ON(__count >= GUC_REGSET_MAX_REGISTERS)) \
+ continue; \
+ node->registers[__count].offset = reg_addr.reg; \
+ node->registers[__count].flags = (_flags); \
+ if (defvalue) \
+ node->registers[__count].value = (defvalue);\
+ node->number_of_registers++;\
+ } while (0)
+
static int guc_ads_create(struct intel_guc *guc)
{
struct drm_i915_private *dev_priv = guc_to_i915(guc);
@@ -1016,6 +1034,7 @@ static int guc_ads_create(struct intel_guc *guc)
u8 reg_state_buffer[GUC_S3_SAVE_SPACE_PAGES * PAGE_SIZE];
} __packed *blob;
struct intel_engine_cs *engine;
+ struct i915_workarounds *workarounds = &dev_priv->workarounds;
enum intel_engine_id id;
u32 base;
@@ -1035,6 +1054,39 @@ static int guc_ads_create(struct intel_guc *guc)
/* MMIO reg state */
for_each_engine(engine, dev_priv, id) {
+ u32 i;
+ struct guc_mmio_regset *eng_reg =
+ &blob->reg_state.engine_reg[engine->guc_id];
+
+ /*
+* Provide a list of registers to be saved/restored during gpu
+* reset. This is mainly required for Media reset (aka watchdog
+* timeout) which is completely under the control of GuC
+* (resubmission of hung workload is handled inside GuC).
+*/
+ GUC_ADD_MMIO_REG_ADS(eng_reg, RING_HWS_PGA(engine->mmio_base),
+GUC_REGSET_ENGINERESET |
+GUC_REGSET_SAVE_CURRENT_VALUE, 0);
+
+ /*
+* Workaround the guc issue with masked registers, note that
+* at this point guc submission is still disabled and the mode
+* register doesnt have the irq_steering bit set, which we
+* need to fwd irqs to GuC.
+*/
+ GUC_ADD_MMIO_REG_ADS(eng_reg, RING_MODE_GEN7(engine),
+GUC_REGSET_ENGINERESET |
+GUC_REGSET_SAVE_DEFAULT_VALUE,
+I915_READ(RING_MODE_GEN7(engine)) |
+GFX_INTERRUPT_STEERING | (0x<<16));
+
+ GUC_ADD_MMIO_REG_ADS(eng_reg, RING_IMR(engine->mmio_base),
+GUC_REGSET_ENGINERESET |
+GUC_REGSET_SAVE_CURRENT_VALUE, 0);
+
+ DRM_DEBUG_DRIVER("%s register save/restore count: %u\n",
+engine->name, eng_reg->number_of_registers);
+
blob->reg_state.white_list[engine->guc_id].mmio_start =
