Re: [Intel-gfx] [PATCH 6/6] drm/xen-front: Add support for EDID based configuration
Hi Oleksandr,
I love your patch! Perhaps something to improve:
[auto build test WARNING on drm-exynos/exynos-drm-next]
[also build test WARNING on drm-intel/for-linux-next
tegra-drm/drm/tegra/for-next drm-tip/drm-tip linus/master v5.8 next-20200804]
[cannot apply to xen-tip/linux-next drm/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Oleksandr-Andrushchenko/Fixes-and-improvements-for-Xen-pvdrm/20200731-205350
base: https://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git
exynos-drm-next
compiler: aarch64-linux-gcc (GCC) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
cppcheck warnings: (new ones prefixed by >>)
>> drivers/irqchip/irq-gic.c:161:24: warning: Local variable gic_data shadows
>> outer variable [shadowVar]
struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
^
drivers/irqchip/irq-gic.c:123:29: note: Shadowed declaration
static struct gic_chip_data gic_data[CONFIG_ARM_GIC_MAX_NR] __read_mostly;
^
drivers/irqchip/irq-gic.c:161:24: note: Shadow variable
struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
^
drivers/irqchip/irq-gic.c:167:24: warning: Local variable gic_data shadows
outer variable [shadowVar]
struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
^
drivers/irqchip/irq-gic.c:123:29: note: Shadowed declaration
static struct gic_chip_data gic_data[CONFIG_ARM_GIC_MAX_NR] __read_mostly;
^
drivers/irqchip/irq-gic.c:167:24: note: Shadow variable
struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
^
>> drivers/irqchip/irq-gic.c:400:28: warning: Local variable gic_irq shadows
>> outer function [shadowFunction]
unsigned int cascade_irq, gic_irq;
^
drivers/irqchip/irq-gic.c:171:28: note: Shadowed declaration
static inline unsigned int gic_irq(struct irq_data *d)
^
drivers/irqchip/irq-gic.c:400:28: note: Shadow variable
unsigned int cascade_irq, gic_irq;
^
>> drivers/irqchip/irq-gic.c:1507:14: warning: Local variable gic_cpu_base
>> shadows outer function [shadowFunction]
phys_addr_t gic_cpu_base;
^
drivers/irqchip/irq-gic.c:165:29: note: Shadowed declaration
static inline void __iomem *gic_cpu_base(struct irq_data *d)
^
drivers/irqchip/irq-gic.c:1507:14: note: Shadow variable
phys_addr_t gic_cpu_base;
^
>> drivers/irqchip/irq-gic-v3.c:874:71: warning: Boolean result is used in
>> bitwise operation. Clarify expression with parentheses. [clarifyCondition]
gic_data.rdists.has_direct_lpi &= (!!(typer & GICR_TYPER_DirectLPIS) |
^
>> drivers/irqchip/irq-gic-v3.c:1808:6: warning: Local variable
>> nr_redist_regions shadows outer variable [shadowVar]
u32 nr_redist_regions;
^
drivers/irqchip/irq-gic-v3.c:1880:6: note: Shadowed declaration
u32 nr_redist_regions;
^
drivers/irqchip/irq-gic-v3.c:1808:6: note: Shadow variable
u32 nr_redist_regions;
^
>> drivers/irqchip/irq-gic-v3.c:2042:6: warning: Local variable maint_irq_mode
>> shadows outer variable [shadowVar]
int maint_irq_mode;
^
drivers/irqchip/irq-gic-v3.c:1884:6: note: Shadowed declaration
int maint_irq_mode;
^
drivers/irqchip/irq-gic-v3.c:2042:6: note: Shadow variable
int maint_irq_mode;
^
>> drivers/gpu/drm/xen/xen_drm_front_cfg.c:76:6: warning: Variable 'ret' is
>> reassigned a value before the old one has been used. [redundantAssignment]
ret = xen_drm_front_get_edid(front_info, index, pages,
^
drivers/gpu/drm/xen/xen_drm_front_cfg.c:61:0: note: Variable 'ret' is
reassigned a value before the old one has been used.
int i, npages, ret = -ENOMEM;
^
drivers/gpu/drm/xen/xen_drm_front_cfg.c:76:6: note: Variable 'ret' is
reassigned a value before the old one has been used.
ret = xen_drm_front_get_edid(front_info, index, pages,
^
vim +/ret +76 drivers/gpu/drm/xen/xen_drm_front_cfg.c
54
55 static void cfg_connector_edid(struct xen_drm_front_info *front_info,
56 struct xen_drm_front_cfg_connector
*connector,
57 int index)
58 {
59 struct page **pages;
60 u32 edid_sz;
61 int i, npages, ret = -ENOMEM;
62
63 connector->edid = vmalloc(XENDISPL_EDID_MAX_SIZE);
64 if (!connector->edid)
65
[PATCH 6/6] drm/xen-front: Add support for EDID based configuration
From: Oleksandr Andrushchenko
Version 2 of the Xen displif protocol adds XENDISPL_OP_GET_EDID
request which allows frontends to request EDID structure per
connector. This request is optional and if not supported by the
backend then visible area is still defined by the relevant
XenStore's "resolution" property.
If backend provides EDID with XENDISPL_OP_GET_EDID request then
its values must take precedence over the resolutions defined in
XenStore.
Signed-off-by: Oleksandr Andrushchenko
---
drivers/gpu/drm/xen/xen_drm_front.c | 62
drivers/gpu/drm/xen/xen_drm_front.h | 9 ++-
drivers/gpu/drm/xen/xen_drm_front_cfg.c | 82 +
drivers/gpu/drm/xen/xen_drm_front_cfg.h | 7 ++
drivers/gpu/drm/xen/xen_drm_front_conn.c| 26 ++-
drivers/gpu/drm/xen/xen_drm_front_evtchnl.c | 3 +
drivers/gpu/drm/xen/xen_drm_front_evtchnl.h | 3 +
drivers/gpu/drm/xen/xen_drm_front_kms.c | 5 ++
8 files changed, 194 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/xen/xen_drm_front.c
b/drivers/gpu/drm/xen/xen_drm_front.c
index 013c9e0e412c..cc5981bdbfb3 100644
--- a/drivers/gpu/drm/xen/xen_drm_front.c
+++ b/drivers/gpu/drm/xen/xen_drm_front.c
@@ -381,6 +381,59 @@ void xen_drm_front_on_frame_done(struct xen_drm_front_info
*front_info,
fb_cookie);
}
+int xen_drm_front_get_edid(struct xen_drm_front_info *front_info,
+ int conn_idx, struct page **pages,
+ u32 buffer_sz, u32 *edid_sz)
+{
+ struct xen_drm_front_evtchnl *evtchnl;
+ struct xen_front_pgdir_shbuf_cfg buf_cfg;
+ struct xen_front_pgdir_shbuf shbuf;
+ struct xendispl_req *req;
+ unsigned long flags;
+ int ret;
+
+ if (unlikely(conn_idx >= front_info->num_evt_pairs))
+ return -EINVAL;
+
+ memset(_cfg, 0, sizeof(buf_cfg));
+ buf_cfg.xb_dev = front_info->xb_dev;
+ buf_cfg.num_pages = DIV_ROUND_UP(buffer_sz, PAGE_SIZE);
+ buf_cfg.pages = pages;
+ buf_cfg.pgdir =
+ buf_cfg.be_alloc = false;
+
+ ret = xen_front_pgdir_shbuf_alloc(_cfg);
+ if (ret < 0)
+ return ret;
+
+ evtchnl = _info->evt_pairs[conn_idx].req;
+
+ mutex_lock(>u.req.req_io_lock);
+
+ spin_lock_irqsave(_info->io_lock, flags);
+ req = be_prepare_req(evtchnl, XENDISPL_OP_GET_EDID);
+ req->op.get_edid.gref_directory =
+ xen_front_pgdir_shbuf_get_dir_start();
+ req->op.get_edid.buffer_sz = buffer_sz;
+
+ ret = be_stream_do_io(evtchnl, req);
+ spin_unlock_irqrestore(_info->io_lock, flags);
+
+ if (ret < 0)
+ goto fail;
+
+ ret = be_stream_wait_io(evtchnl);
+ if (ret < 0)
+ goto fail;
+
+ *edid_sz = evtchnl->u.req.resp.get_edid.edid_sz;
+
+fail:
+ mutex_unlock(>u.req.req_io_lock);
+ xen_front_pgdir_shbuf_free();
+ return ret;
+}
+
static int xen_drm_drv_dumb_create(struct drm_file *filp,
struct drm_device *dev,
struct drm_mode_create_dumb *args)
@@ -466,6 +519,7 @@ static void xen_drm_drv_release(struct drm_device *dev)
xenbus_switch_state(front_info->xb_dev,
XenbusStateInitialising);
+ xen_drm_front_cfg_free(front_info, _info->cfg);
kfree(drm_info);
}
@@ -562,6 +616,7 @@ static int xen_drm_drv_init(struct xen_drm_front_info
*front_info)
drm_mode_config_cleanup(drm_dev);
drm_dev_put(drm_dev);
fail:
+ xen_drm_front_cfg_free(front_info, _info->cfg);
kfree(drm_info);
return ret;
}
@@ -622,7 +677,14 @@ static int displback_initwait(struct xen_drm_front_info
*front_info)
static int displback_connect(struct xen_drm_front_info *front_info)
{
+ int ret;
+
xen_drm_front_evtchnl_set_state(front_info, EVTCHNL_STATE_CONNECTED);
+
+ /* We are all set to read additional configuration from the backend. */
+ ret = xen_drm_front_cfg_tail(front_info, _info->cfg);
+ if (ret < 0)
+ return ret;
return xen_drm_drv_init(front_info);
}
diff --git a/drivers/gpu/drm/xen/xen_drm_front.h
b/drivers/gpu/drm/xen/xen_drm_front.h
index 54486d89650e..be0c982f4d82 100644
--- a/drivers/gpu/drm/xen/xen_drm_front.h
+++ b/drivers/gpu/drm/xen/xen_drm_front.h
@@ -112,9 +112,12 @@ struct xen_drm_front_drm_pipeline {
struct drm_simple_display_pipe pipe;
struct drm_connector conn;
- /* These are only for connector mode checking */
+ /* These are only for connector mode checking if no EDID present */
int width, height;
+ /* Is not NULL if EDID is used for connector configuration. */
+ struct edid *edid;
+
struct drm_pending_vblank_event *pending_event;
struct delayed_work pflip_to_worker;
@@ -160,4 +163,8 @@ int
