Re: [PATCH v3] misc: fastrpc: Add support for multiple PD from one process
Thanks Ekansh for the patch,
On 08/08/2024 11:42, Ekansh Gupta wrote:
Memory intensive applications(which requires more tha 4GB) that wants
to offload tasks to DSP might have to split the tasks to multiple
user PD to make the resources available.
For every call to DSP, fastrpc driver passes the process tgid which
works as an identifier for the DSP to enqueue the tasks to specific PD.
With current design, if any process opens device node more than once
and makes PD init request, same tgid will be passed to DSP which will
be considered a bad request and this will result in failure as the same
identifier cannot be used for multiple DSP PD.
Assign and pass a client ID to DSP which would be assigned during device
open and will be dependent on the index of session allocated for the PD.
This will allow the same process to open the device more than once and
spawn multiple dynamic PD for ease of processing.
Signed-off-by: Ekansh Gupta
---
Changes in v2:
- Reformatted commit text.
- Moved from ida to idr.
- Changed dsp_pgid data type.
- Resolved memory leak.
Changes in v3:
- Modified commit text.
- Removed idr implementation.
- Using session index for client id.
drivers/misc/fastrpc.c | 30 --
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index a7a2bcedb37e..0ce1eedcb2c3 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -38,6 +38,7 @@
#define FASTRPC_INIT_HANDLE 1
#define FASTRPC_DSP_UTILITIES_HANDLE 2
#define FASTRPC_CTXID_MASK (0xFF0)
+#define FASTRPC_CLIENTID_MASK (16)
MASK normally is a hex mask rather than integer.
Looking at the actual code changes this looks like a flag rather than a
mask. This is setting up bit 5 of the process group id in struct
fastrpc_msg and other structs.
This really needs some documentation of how the group id is partitioned
to have these various fields.
Something like:
/* FastRPC Group ID fields*/
#define FASTRPC_GID_SESSION_ID_MASK GENMASK(3, 0)
#define FASTRPC_GID_CLIENT_ID_MASK GENMASK(4, 4)
#define INIT_FILELEN_MAX (2 * 1024 * 1024)
#define INIT_FILE_NAMELEN_MAX (128)
#define FASTRPC_DEVICE_NAME "fastrpc"
@@ -298,7 +299,7 @@ struct fastrpc_user {
struct fastrpc_session_ctx *sctx;
struct fastrpc_buf *init_mem;
- int tgid;
+ int client_id;
Can you we rename this as groupid, this clearly reflects the facts that
group id has multiple things as described above.
If group id is not the correct name instead it should be client_id, then
this should also be reflected in various sturcts that have group id as
tgid, pid, pgid an so.. that is another cleanup which should go as new
patch.
--srini
int pd;
bool is_secure_dev;
/* Lock for lists */
@@ -613,7 +614,7 @@ static struct fastrpc_invoke_ctx *fastrpc_context_alloc(
ctx->sc = sc;
ctx->retval = -1;
ctx->pid = current->pid;
- ctx->tgid = user->tgid;
+ ctx->tgid = user->client_id;
ctx->cctx = cctx;
init_completion(&ctx->work);
INIT_WORK(&ctx->put_work, fastrpc_context_put_wq);
@@ -,7 +1112,7 @@ static int fastrpc_invoke_send(struct fastrpc_session_ctx
*sctx,
int ret;
cctx = fl->cctx;
- msg->pid = fl->tgid;
+ msg->pid = fl->client_id;
msg->tid = current->pid;
if (kernel)
@@ -1294,7 +1295,7 @@ static int fastrpc_init_create_static_process(struct
fastrpc_user *fl,
}
}
- inbuf.pgid = fl->tgid;
+ inbuf.pgid = fl->client_id;
inbuf.namelen = init.namelen;
inbuf.pageslen = 0;
fl->pd = USER_PD;
@@ -1396,7 +1397,7 @@ static int fastrpc_init_create_process(struct
fastrpc_user *fl,
goto err;
}
- inbuf.pgid = fl->tgid;
+ inbuf.pgid = fl->client_id;
inbuf.namelen = strlen(current->comm) + 1;
inbuf.filelen = init.filelen;
inbuf.pageslen = 1;
@@ -1470,8 +1471,9 @@ static int fastrpc_init_create_process(struct
fastrpc_user *fl,
}
static struct fastrpc_session_ctx *fastrpc_session_alloc(
- struct fastrpc_channel_ctx *cctx)
+ struct fastrpc_user *fl)
{
+ struct fastrpc_channel_ctx *cctx = fl->cctx;
struct fastrpc_session_ctx *session = NULL;
unsigned long flags;
int i;
@@ -1481,6 +1483,7 @@ static struct fastrpc_session_ctx *fastrpc_session_alloc(
if (!cctx->session[i].used && cctx->session[i].valid) {
cctx->session[i].used = true;
session = &cctx->session[i];
+ fl->client_id = FASTRPC_CLIENTID_MASK | i;
break;
}
}
@@ -1505,7 +1508,7 @@ static int fastrpc_release_current_dsp_process(struct
fastrpc_user *fl)
int tgid = 0;
u32 sc;
- tgid = fl->
Re: (subset) [PATCH v4 0/6] arm64: qcom: sa8775p: enable remoteprocs - ADSP, CDSP and GPDSP
On Mon, 05 Aug 2024 19:08:01 +0200, Bartosz Golaszewski wrote: > Add DT bindings, relevant DT defines, DTS nodes and driver changes > required to enable the remoteprocs on sa8775p. > > To: Bjorn Andersson > To: Mathieu Poirier > To: Rob Herring > To: Krzysztof Kozlowski > To: Conor Dooley > To: Manivannan Sadhasivam > To: Jassi Brar > To: Konrad Dybcio > Cc: linux-arm-...@vger.kernel.org > Cc: linux-remotep...@vger.kernel.org > Cc: devicet...@vger.kernel.org > Cc: linux-ker...@vger.kernel.org > Signed-off-by: Bartosz Golaszewski > > [...] Applied, thanks! [1/6] dt-bindings: misc: qcom,fastrpc: increase the max number of iommus commit: 42a21d00aac515fad1f9a10052c6e9710c6f7813 [4/6] misc: fastrpc: Add support for cdsp1 remoteproc commit: 590c42d9e278f8e6bf6d673f3101ac102369efc7 Best regards, -- Srinivas Kandagatla
Re: [PATCH] Revert "misc: fastrpc: Restrict untrusted app to attach to privileged PD"
On 15/08/2024 10:49, Griffin Kroah-Hartman wrote:
This reverts commit bab2f5e8fd5d2f759db26b78d9db57412888f187.
Joel reported that this commit breaks userspace and stops sensors in
SDM845 from working. Also breaks other qcom SoC devices running postmarketOS.
Cc: stable
Cc: Ekansh Gupta
Cc: Dmitry Baryshkov
Cc: Srinivas Kandagatla
Cc: Greg Kroah-Hartman
Reported-by: Joel Selvaraj
Link:
https://lore.kernel.org/r/9a9f5646-a554-4b65-8122-d212bb665...@umsystem.edu
Signed-off-by: Griffin Kroah-Hartman
---
Lets fix this properly by updating existing device tree entries and
bindings to enforce misuse of "qcom,non-secure-domain" on all Qcom SoC's.
As this patch is only doing part of the job and breaking existing
platforms,
Acked-by: Srinivas Kandagatla
--srini
--srini
drivers/misc/fastrpc.c | 22 +++---
include/uapi/misc/fastrpc.h | 3 ---
2 files changed, 3 insertions(+), 22 deletions(-)
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 5204fda51da3..339d126414d4 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -2085,16 +2085,6 @@ static int fastrpc_req_mem_map(struct fastrpc_user *fl,
char __user *argp)
return err;
}
-static int is_attach_rejected(struct fastrpc_user *fl)
-{
- /* Check if the device node is non-secure */
- if (!fl->is_secure_dev) {
- dev_dbg(&fl->cctx->rpdev->dev, "untrusted app trying to attach to
privileged DSP PD\n");
- return -EACCES;
- }
- return 0;
-}
-
static long fastrpc_device_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
{
@@ -2107,19 +2097,13 @@ static long fastrpc_device_ioctl(struct file *file,
unsigned int cmd,
err = fastrpc_invoke(fl, argp);
break;
case FASTRPC_IOCTL_INIT_ATTACH:
- err = is_attach_rejected(fl);
- if (!err)
- err = fastrpc_init_attach(fl, ROOT_PD);
+ err = fastrpc_init_attach(fl, ROOT_PD);
break;
case FASTRPC_IOCTL_INIT_ATTACH_SNS:
- err = is_attach_rejected(fl);
- if (!err)
- err = fastrpc_init_attach(fl, SENSORS_PD);
+ err = fastrpc_init_attach(fl, SENSORS_PD);
break;
case FASTRPC_IOCTL_INIT_CREATE_STATIC:
- err = is_attach_rejected(fl);
- if (!err)
- err = fastrpc_init_create_static_process(fl, argp);
+ err = fastrpc_init_create_static_process(fl, argp);
break;
case FASTRPC_IOCTL_INIT_CREATE:
err = fastrpc_init_create_process(fl, argp);
diff --git a/include/uapi/misc/fastrpc.h b/include/uapi/misc/fastrpc.h
index 91583690bddc..f33d914d8f46 100644
--- a/include/uapi/misc/fastrpc.h
+++ b/include/uapi/misc/fastrpc.h
@@ -8,14 +8,11 @@
#define FASTRPC_IOCTL_ALLOC_DMA_BUFF _IOWR('R', 1, struct
fastrpc_alloc_dma_buf)
#define FASTRPC_IOCTL_FREE_DMA_BUFF _IOWR('R', 2, __u32)
#define FASTRPC_IOCTL_INVOKE _IOWR('R', 3, struct fastrpc_invoke)
-/* This ioctl is only supported with secure device nodes */
#define FASTRPC_IOCTL_INIT_ATTACH _IO('R', 4)
#define FASTRPC_IOCTL_INIT_CREATE _IOWR('R', 5, struct
fastrpc_init_create)
#define FASTRPC_IOCTL_MMAP_IOWR('R', 6, struct fastrpc_req_mmap)
#define FASTRPC_IOCTL_MUNMAP _IOWR('R', 7, struct fastrpc_req_munmap)
-/* This ioctl is only supported with secure device nodes */
#define FASTRPC_IOCTL_INIT_ATTACH_SNS _IO('R', 8)
-/* This ioctl is only supported with secure device nodes */
#define FASTRPC_IOCTL_INIT_CREATE_STATIC _IOWR('R', 9, struct
fastrpc_init_create_static)
#define FASTRPC_IOCTL_MEM_MAP _IOWR('R', 10, struct fastrpc_mem_map)
#define FASTRPC_IOCTL_MEM_UNMAP _IOWR('R', 11, struct
fastrpc_mem_unmap)
Re: [PATCH] MAINTAINERS: CC dri-devel list on Qualcomm FastRPC patches
On Mon, 24 Jun 2024 11:19:09 +0300, Dmitry Baryshkov wrote: > FastRPC is a way to offload method invocation to the DSPs on Qualcomm > platforms. As the driver uses dma-bufs, add dri-devel mailing list to > the MAINTAINERS's entry, so that DRM maintainers are notified about the > uAPI changes. This follows the usual practice established by the "DMA > BUFFER SHARING FRAMEWORK" entry in the file. > > > [...] Applied, thanks! [1/1] MAINTAINERS: CC dri-devel list on Qualcomm FastRPC patches commit: 47bf4198bf386018e99673c1ce5dbd8e5eda293e Best regards, -- Srinivas Kandagatla
Re: (subset) [PATCH v2 0/7] Move dma-buf mmap() reservation locking down to exporters
On Thu, 06 Apr 2023 19:06:30 +0300, Dmitry Osipenko wrote: > This patchset makes dma-buf exporters responisble for taking care of > the reservation lock. I also included patch that moves drm-shmem to use > reservation lock, to let CI test the whole set. I'm going to take all > the patches via the drm-misc tree, please give an ack. > > Previous policy stated that dma-buf core takes the lock around mmap() > callback. Which meant that both importers and exporters shouldn't touch > the reservation lock in the mmap() code path. This worked well until > Intel-CI found a deadlock problem in a case of self-imported dma-buf [1]. > > [...] Applied, thanks! [4/7] fastrpc: Don't assert held reservation lock for dma-buf mmapping commit: 3f6b4c6f0bd0126f673f3578429239ae3860718b Best regards, -- Srinivas Kandagatla
[PATCH] drm/msm/dp: unregister audio driver during unbind
while binding the code always registers a audio driver, however there
is no corresponding unregistration done in unbind. This leads to multiple
redundant audio platform devices if dp_display_bind and dp_display_unbind
happens multiple times during startup. On X13s platform this resulted in
6 to 9 audio codec device instead of just 3 codec devices for 3 dp ports.
Fix this by unregistering codecs on unbind.
Signed-off-by: Srinivas Kandagatla
---
drivers/gpu/drm/msm/dp/dp_audio.c | 12
drivers/gpu/drm/msm/dp/dp_audio.h | 2 ++
drivers/gpu/drm/msm/dp/dp_display.c | 1 +
3 files changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/msm/dp/dp_audio.c
b/drivers/gpu/drm/msm/dp/dp_audio.c
index 783e1468..1245c7aa49df 100644
--- a/drivers/gpu/drm/msm/dp/dp_audio.c
+++ b/drivers/gpu/drm/msm/dp/dp_audio.c
@@ -593,6 +593,18 @@ static struct hdmi_codec_pdata codec_data = {
.i2s = 1,
};
+void dp_unregister_audio_driver(struct device *dev, struct dp_audio *dp_audio)
+{
+ struct dp_audio_private *audio_priv;
+
+ audio_priv = container_of(dp_audio, struct dp_audio_private, dp_audio);
+
+ if (audio_priv->audio_pdev) {
+ platform_device_unregister(audio_priv->audio_pdev);
+ audio_priv->audio_pdev = NULL;
+ }
+}
+
int dp_register_audio_driver(struct device *dev,
struct dp_audio *dp_audio)
{
diff --git a/drivers/gpu/drm/msm/dp/dp_audio.h
b/drivers/gpu/drm/msm/dp/dp_audio.h
index 84e5f4a5d26b..4ab78880af82 100644
--- a/drivers/gpu/drm/msm/dp/dp_audio.h
+++ b/drivers/gpu/drm/msm/dp/dp_audio.h
@@ -53,6 +53,8 @@ struct dp_audio *dp_audio_get(struct platform_device *pdev,
int dp_register_audio_driver(struct device *dev,
struct dp_audio *dp_audio);
+void dp_unregister_audio_driver(struct device *dev, struct dp_audio *dp_audio);
+
/**
* dp_audio_put()
*
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 3e13acdfa7e5..99a38dbe51c0 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -326,6 +326,7 @@ static void dp_display_unbind(struct device *dev, struct
device *master,
kthread_stop(dp->ev_tsk);
dp_power_client_deinit(dp->power);
+ dp_unregister_audio_driver(dev, dp->audio);
dp_aux_unregister(dp->aux);
dp->drm_dev = NULL;
dp->aux->drm_dev = NULL;
--
2.21.0
Re: [PATCH v4 11/21] misc: fastrpc: Prepare to dynamic dma-buf locking specification
On 31/08/2022 16:37, Dmitry Osipenko wrote:
Prepare fastrpc to the common dynamic dma-buf locking convention by
starting to use the unlocked versions of dma-buf API functions.
Signed-off-by: Dmitry Osipenko
---
LGTM,
Incase you plan to take it via another tree.
Acked-by: Srinivas Kandagatla
--srini
drivers/misc/fastrpc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 93ebd174d848..6fcfb2e9f7a7 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -310,8 +310,8 @@ static void fastrpc_free_map(struct kref *ref)
return;
}
}
- dma_buf_unmap_attachment(map->attach, map->table,
-DMA_BIDIRECTIONAL);
+ dma_buf_unmap_attachment_unlocked(map->attach, map->table,
+ DMA_BIDIRECTIONAL);
dma_buf_detach(map->buf, map->attach);
dma_buf_put(map->buf);
}
@@ -726,7 +726,7 @@ static int fastrpc_map_create(struct fastrpc_user *fl, int
fd,
goto attach_err;
}
- map->table = dma_buf_map_attachment(map->attach, DMA_BIDIRECTIONAL);
+ map->table = dma_buf_map_attachment_unlocked(map->attach,
DMA_BIDIRECTIONAL);
if (IS_ERR(map->table)) {
err = PTR_ERR(map->table);
goto map_err;
Re: [PATCH v4 11/21] misc: fastrpc: Prepare to dynamic dma-buf locking specification
On 31/08/2022 16:37, Dmitry Osipenko wrote:
Prepare fastrpc to the common dynamic dma-buf locking convention by
starting to use the unlocked versions of dma-buf API functions.
Signed-off-by: Dmitry Osipenko
---
LGTM,
Incase you plan to take it via another tree.
Acked-by: Srinivas Kandagatla
--srini
drivers/misc/fastrpc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 93ebd174d848..6fcfb2e9f7a7 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -310,8 +310,8 @@ static void fastrpc_free_map(struct kref *ref)
return;
}
}
- dma_buf_unmap_attachment(map->attach, map->table,
-DMA_BIDIRECTIONAL);
+ dma_buf_unmap_attachment_unlocked(map->attach, map->table,
+ DMA_BIDIRECTIONAL);
dma_buf_detach(map->buf, map->attach);
dma_buf_put(map->buf);
}
@@ -726,7 +726,7 @@ static int fastrpc_map_create(struct fastrpc_user *fl, int
fd,
goto attach_err;
}
- map->table = dma_buf_map_attachment(map->attach, DMA_BIDIRECTIONAL);
+ map->table = dma_buf_map_attachment_unlocked(map->attach,
DMA_BIDIRECTIONAL);
if (IS_ERR(map->table)) {
err = PTR_ERR(map->table);
goto map_err;
Re: [PATCH RESEND - dt 1/2] dt-bindings: nvmem: qfprom: add IPQ8064 and SDM630 compatibles
Hi Krzysztof, I seems to have missed this one, On 25/08/2022 13:54, Krzysztof Kozlowski wrote: Document compatibles for QFPROM used on IPQ8064 and SDM630. They are compatible with generic QFPROM fallback. Signed-off-by: Krzysztof Kozlowski Applied thanks, Srini --- Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml b/Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml index dede8892ee01..b4163086a5be 100644 --- a/Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml +++ b/Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml @@ -18,6 +18,7 @@ properties: - enum: - qcom,apq8064-qfprom - qcom,apq8084-qfprom + - qcom,ipq8064-qfprom - qcom,msm8974-qfprom - qcom,msm8916-qfprom - qcom,msm8996-qfprom @@ -25,6 +26,7 @@ properties: - qcom,qcs404-qfprom - qcom,sc7180-qfprom - qcom,sc7280-qfprom + - qcom,sdm630-qfprom - qcom,sdm845-qfprom - const: qcom,qfprom
Re: [PATCH 1/3] drm/msm: Fix speed-bin support not to access outside valid memory
On 05/03/2021 14:45, Doug Anderson wrote:
Hi,
On Fri, Mar 5, 2021 at 2:28 AM Srinivas Kandagatla
wrote:
On 27/02/2021 00:26, Douglas Anderson wrote:
When running the latest kernel on an sc7180 with KASAN I got this
splat:
BUG: KASAN: slab-out-of-bounds in a6xx_gpu_init+0x618/0x644
Read of size 4 at addr ff8088f36100 by task kworker/7:1/58
CPU: 7 PID: 58 Comm: kworker/7:1 Not tainted 5.11.0+ #3
Hardware name: Google Lazor (rev1 - 2) with LTE (DT)
Workqueue: events deferred_probe_work_func
Call trace:
dump_backtrace+0x0/0x3a8
show_stack+0x24/0x30
dump_stack+0x174/0x1e0
print_address_description+0x70/0x2e4
kasan_report+0x178/0x1bc
__asan_report_load4_noabort+0x44/0x50
a6xx_gpu_init+0x618/0x644
adreno_bind+0x26c/0x438
This is because the speed bin is defined like this:
gpu_speed_bin: gpu_speed_bin@1d2 {
reg = <0x1d2 0x2>;
bits = <5 8>;
};
As you can see the "length" is 2 bytes. That means that the nvmem
subsystem allocates only 2 bytes. The GPU code, however, was casting
the pointer allocated by nvmem to a (u32 *) and dereferencing. That's
not so good.
Let's fix this to just use the nvmem_cell_read_u16() accessor function
which simplifies things and also gets rid of the splat.
Let's also put an explicit conversion from little endian in place just
to make things clear. The nvmem subsystem today is assuming little
endian and this makes it clear. Specifically, the way the above sc7180
cell is interpreted:
NVMEM:
++++++
| .. | 0x1d3 | 0x1d2 | .. | 0x000 |
++++++
^ ^
msb lsb
You can see that the least significant data is at the lower address
which is little endian.
NOTE: someone who is truly paying attention might wonder about me
picking the "u16" version of this accessor instead of the "u8" (since
the value is 8 bits big) or the u32 version (just for fun). At the
moment you need to pick the accessor that exactly matches the length
the cell was specified as in the device tree. Hopefully future
patches to the nvmem subsystem will fix this.
Fixes: fe7952c629da ("drm/msm: Add speed-bin support to a618 gpu")
Signed-off-by: Douglas Anderson
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 31 +++
1 file changed, 8 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index ba8e9d3cf0fe..0e2024defd79 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1350,35 +1350,20 @@ static int a6xx_set_supported_hw(struct device *dev,
struct a6xx_gpu *a6xx_gpu,
u32 revn)
{
struct opp_table *opp_table;
- struct nvmem_cell *cell;
u32 supp_hw = UINT_MAX;
- void *buf;
-
- cell = nvmem_cell_get(dev, "speed_bin");
- /*
- * -ENOENT means that the platform doesn't support speedbin which is
- * fine
- */
- if (PTR_ERR(cell) == -ENOENT)
- return 0;
- else if (IS_ERR(cell)) {
- DRM_DEV_ERROR(dev,
- "failed to read speed-bin. Some OPPs may not be
supported by hardware");
- goto done;
- }
+ u16 speedbin;
+ int ret;
- buf = nvmem_cell_read(cell, NULL);
I think the issue here is not passing len pointer which should return
how many bytes the cell is!
Then from there we can decide to do le16_to_cpu or le32_to_cpu or not!
This will also future proof the code to handle speed_bins of different
sizes!
I think what you're saying is that you want to copy/paste this code
(or something similar) everywhere that accesses an nvmem cell. Is
that correct? ...or maybe you can suggest some smaller / shorter code
that I'm missing?
It depends what the consumer is doing! If it is already aware of what
size of data its expecting then you can use nvmem_cell_read_u8/16/32/64
variants, however it wants to do bit more with the data then
nvmem_cell_read() should give more flexibility!
---
{
struct nvmem_cell *cell;
ssize_t len;
char *ret;
int i;
*data = 0;
cell = nvmem_cell_get(dev, cname);
if (IS_ERR(cell)) {
if (PTR_ERR(cell) != -EPROBE_DEFER)
dev_err(dev, "undefined cell %s\n", cname);
return PTR_ERR(cell);
}
ret = nvmem_cell_read(cell, &len);
nvmem_cell_put(cell);
if (IS_ERR(ret)) {
dev_err(dev, "can't read cell %s\n", cname);
return PTR_ERR(ret);
}
for (i = 0; i < len; i++)
*data |= ret[i] << (8 * i);
kfree(ret);
dev_dbg(dev, "efuse read(%s) = %x, bytes %zd\n", cname, *data, len);
return 0;
}
---
The above code is from cpr_read_efuse() in "cpr.c". I mentioned in
the c
Re: [PATCH 1/3] drm/msm: Fix speed-bin support not to access outside valid memory
On 27/02/2021 00:26, Douglas Anderson wrote:
When running the latest kernel on an sc7180 with KASAN I got this
splat:
BUG: KASAN: slab-out-of-bounds in a6xx_gpu_init+0x618/0x644
Read of size 4 at addr ff8088f36100 by task kworker/7:1/58
CPU: 7 PID: 58 Comm: kworker/7:1 Not tainted 5.11.0+ #3
Hardware name: Google Lazor (rev1 - 2) with LTE (DT)
Workqueue: events deferred_probe_work_func
Call trace:
dump_backtrace+0x0/0x3a8
show_stack+0x24/0x30
dump_stack+0x174/0x1e0
print_address_description+0x70/0x2e4
kasan_report+0x178/0x1bc
__asan_report_load4_noabort+0x44/0x50
a6xx_gpu_init+0x618/0x644
adreno_bind+0x26c/0x438
This is because the speed bin is defined like this:
gpu_speed_bin: gpu_speed_bin@1d2 {
reg = <0x1d2 0x2>;
bits = <5 8>;
};
As you can see the "length" is 2 bytes. That means that the nvmem
subsystem allocates only 2 bytes. The GPU code, however, was casting
the pointer allocated by nvmem to a (u32 *) and dereferencing. That's
not so good.
Let's fix this to just use the nvmem_cell_read_u16() accessor function
which simplifies things and also gets rid of the splat.
Let's also put an explicit conversion from little endian in place just
to make things clear. The nvmem subsystem today is assuming little
endian and this makes it clear. Specifically, the way the above sc7180
cell is interpreted:
NVMEM:
++++++
| .. | 0x1d3 | 0x1d2 | .. | 0x000 |
++++++
^ ^
msb lsb
You can see that the least significant data is at the lower address
which is little endian.
NOTE: someone who is truly paying attention might wonder about me
picking the "u16" version of this accessor instead of the "u8" (since
the value is 8 bits big) or the u32 version (just for fun). At the
moment you need to pick the accessor that exactly matches the length
the cell was specified as in the device tree. Hopefully future
patches to the nvmem subsystem will fix this.
Fixes: fe7952c629da ("drm/msm: Add speed-bin support to a618 gpu")
Signed-off-by: Douglas Anderson
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 31 +++
1 file changed, 8 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index ba8e9d3cf0fe..0e2024defd79 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1350,35 +1350,20 @@ static int a6xx_set_supported_hw(struct device *dev,
struct a6xx_gpu *a6xx_gpu,
u32 revn)
{
struct opp_table *opp_table;
- struct nvmem_cell *cell;
u32 supp_hw = UINT_MAX;
- void *buf;
-
- cell = nvmem_cell_get(dev, "speed_bin");
- /*
-* -ENOENT means that the platform doesn't support speedbin which is
-* fine
-*/
- if (PTR_ERR(cell) == -ENOENT)
- return 0;
- else if (IS_ERR(cell)) {
- DRM_DEV_ERROR(dev,
- "failed to read speed-bin. Some OPPs may not be
supported by hardware");
- goto done;
- }
+ u16 speedbin;
+ int ret;
- buf = nvmem_cell_read(cell, NULL);
I think the issue here is not passing len pointer which should return
how many bytes the cell is!
Then from there we can decide to do le16_to_cpu or le32_to_cpu or not!
This will also future proof the code to handle speed_bins of different
sizes!
--srini
- if (IS_ERR(buf)) {
- nvmem_cell_put(cell);
+ ret = nvmem_cell_read_u16(dev, "speed_bin", &speedbin);
+ if (ret) {
DRM_DEV_ERROR(dev,
- "failed to read speed-bin. Some OPPs may not be
supported by hardware");
+ "failed to read speed-bin (%d). Some OPPs may not be
supported by hardware",
+ ret);
goto done;
}
+ speedbin = le16_to_cpu(speedbin);
- supp_hw = fuse_to_supp_hw(dev, revn, *((u32 *) buf));
-
- kfree(buf);
- nvmem_cell_put(cell);
+ supp_hw = fuse_to_supp_hw(dev, revn, speedbin);
done:
opp_table = dev_pm_opp_set_supported_hw(dev, &supp_hw, 1);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 19/39] docs: ABI: stable: make files ReST compatible
On 30/10/2020 07:40, Mauro Carvalho Chehab wrote: Several entries at the stable ABI files won't parse if we pass them directly to the ReST output. Adjust them, in order to allow adding their contents as-is at the stable ABI book. Signed-off-by: Mauro Carvalho Chehab --- Documentation/ABI/stable/firewire-cdev| 4 + Documentation/ABI/stable/sysfs-acpi-pmprofile | 22 +++-- Documentation/ABI/stable/sysfs-bus-firewire | 3 + Documentation/ABI/stable/sysfs-bus-nvmem | 19 ++-- for nvmem parts: Acked-by: Srinivas Kandagatla --srini Documentation/ABI/stable/sysfs-bus-usb| 6 +- .../ABI/stable/sysfs-class-backlight | 1 + .../ABI/stable/sysfs-class-infiniband | 93 +-- Documentation/ABI/stable/sysfs-class-rfkill | 13 ++- Documentation/ABI/stable/sysfs-class-tpm | 90 +- Documentation/ABI/stable/sysfs-devices| 5 +- Documentation/ABI/stable/sysfs-driver-ib_srp | 1 + .../ABI/stable/sysfs-firmware-efi-vars| 4 + .../ABI/stable/sysfs-firmware-opal-dump | 5 + .../ABI/stable/sysfs-firmware-opal-elog | 2 + Documentation/ABI/stable/sysfs-hypervisor-xen | 3 + Documentation/ABI/stable/vdso | 5 +- 16 files changed, 176 insertions(+), 100 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/4] dt-bindings: additional/unevaluatedProperties clean-ups
On 05/10/2020 19:38, Rob Herring wrote: The default behavior for json-schema is any unknown property is allowed. T hat is generally not the behavior we want for DT. In order to disallow extra properties, schemas need to define 'additionalProperties: false' typically. Ideally, we'd just add that automatically with the tools, but there are some exceptions so only making things explicit everywhere really works. Missing 'additionalProperties' or 'unevaluatedProperties' has been a constant source of review comments, so a meta-schema check is really needed here. Documentation/devicetree/bindings/nvmem/nvmem.yaml | 2 ++ .../devicetree/bindings/nvmem/qcom,qfprom.yaml | 2 ++ for nvmem parts, Acked-by: Srinivas Kandagatla thanks, --srini ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] dt-bindings: Explicitly allow additional properties in common schemas
On 05/10/2020 19:38, Rob Herring wrote: In order to add meta-schema checks for additional/unevaluatedProperties being present, all schema need to make this explicit. As common/shared schema are included by other schemas, they should always allow for additionalProperties. Signed-off-by: Rob Herring Documentation/devicetree/bindings/nvmem/nvmem-consumer.yaml | 2 ++ Documentation/devicetree/bindings/nvmem/nvmem.yaml | 2 ++ for nvmem parts, Acked-by: Srinivas Kandagatla thanks, srini ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [alsa-devel] [PATCH v2 2/2] drm/bridge: adv7511: restrict audio sample sizes
On 01/08/17 13:28, Mark Brown wrote: On Tue, Aug 01, 2017 at 01:24:03PM +0100, Srinivas Kandagatla wrote: On 01/08/17 09:42, Arnaud Pouliquen wrote: On 08/01/2017 12:49 AM, srinivas.kandaga...@linaro.org wrote: I already used ADV7513 and i did not observe this limitation. I had a look to ADV7533 data-sheet. it should support 32 and 64 bits I2S ADV7511_REG_AUDIO_CFG3(0x14) register definition in datasheet and the code in this driver suggest that It only supports 16Bit and 24Bit samples. The amount of data it pays attention to in the frame is not the same as the size of the frame. I agree. format bus, with 16 or 24 bits precision sample. So it should support SNDRV_PCM_FMTBIT_S32_LE and SNDRV_PCM_FMTBIT_S32_BE As example, if you configure bus in Left justified format with 24 bits sample length, 32 bits application samples should be truncated to 24 bits samples at ADV7533 I2S interface level (LSB dropped). May be we can do that to make the user happy but isn't this just truncate the resolution to 24Bit then? And it's a false indication that we are supporting 32bit samples. Which am not very happy with. This is what the sample_bits field in the DAI structure is for.ya. But still reporting that driver supports 32 bit samples when it does not really support all 32 bits, is kinda misleading to user. Isn't it? And the driver would be end up with hacked up code for each case. --srini ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [alsa-devel] [PATCH v2 2/2] drm/bridge: adv7511: restrict audio sample sizes
On 01/08/17 09:42, Arnaud Pouliquen wrote:
Hello Srinivas,
On 08/01/2017 12:49 AM, srinivas.kandaga...@linaro.org wrote:
From: Srinivas Kandagatla
ADV7533 only supports audio samples word width from 16-24 bits.
This patch restricts the audio sample sizes to the supported ones,
so that sound card does not report wrong list of supported hwparms.
Without this patch aplay would fail when playing a 32 bit audio.
Signed-off-by: Srinivas Kandagatla
---
drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
index 67469c26bae8..d01d0aa0eef7 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
@@ -214,6 +214,8 @@ static struct hdmi_codec_pdata codec_data = {
.ops = &adv7511_codec_ops,
.max_i2s_channels = 2,
.i2s = 1,
+ .formats = (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE |
+ SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S24_BE),
};
I'm not sure that this limitation is needed.
I already used ADV7513 and i did not observe this limitation.
I had a look to ADV7533 data-sheet. it should support 32 and 64 bits I2S
ADV7511_REG_AUDIO_CFG3(0x14) register definition in datasheet and the
code in this driver suggest that It only supports 16Bit and 24Bit samples.
format bus, with 16 or 24 bits precision sample. So it should support
SNDRV_PCM_FMTBIT_S32_LE and SNDRV_PCM_FMTBIT_S32_BE
As example, if you configure bus in Left justified format with 24 bits
sample length, 32 bits application samples should be truncated to 24
bits samples at ADV7533 I2S interface level (LSB dropped).
May be we can do that to make the user happy but isn't this just
truncate the resolution to 24Bit then?
And it's a false indication that we are supporting 32bit samples.
Which am not very happy with.
Perhaps i missed something... but look like more an I2S bus
configuration issue in your DT card description, that that a miss in the
drivers.
Nothing to do with DT or other driver, adv7533 audio driver does not
support 32 bit sample size which is why it would return -EINVAL from
adv7511_hdmi_hw_params().
This patch is fixing the loose ends, by which I mean the card would
report the exact supported sample sizes to user rather than every thing
in the I2S_FORMATS list.
Thanks,
srini
Regards
Arnaud
int adv7511_audio_init(struct device *dev, struct adv7511 *adv7511)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/2] ASoC: hdmi-codec: Allow driver to restrict sample size
From: Srinivas Kandagatla This patch adds option to allow hdmi-codec drivers to restrict the audio sample size based on the values that are suppored in hardware. First patch is adding that formats option to hdmi_codec_pdata and second patch is a fix in adv7511 codec driver which only supports 16 and 24 bit samples. Changes since v1: - Added Review by Jyri - Updated comment in code suggested by Jyri Srinivas Kandagatla (2): ASoC: hdmi-codec: Allow drivers to restrict sample sizes. drm/bridge: adv7511: restrict audio sample sizes drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 2 ++ include/sound/hdmi-codec.h | 1 + sound/soc/codecs/hdmi-codec.c | 8 +--- 3 files changed, 8 insertions(+), 3 deletions(-) -- 2.11.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/2] ASoC: hdmi-codec: Allow drivers to restrict sample sizes.
From: Srinivas Kandagatla
Currently hdmi client drivers does have means to limit the
sample sizes that it can only support. Having formats parameter
option would solve this.
This issue was noticed on DB410c board when adv7511 hdmi codec driver
failed to play a 32 bits audio samples, as it does not support them.
Signed-off-by: Srinivas Kandagatla
Reviewed-by: Jyri Sarha
---
include/sound/hdmi-codec.h| 1 +
sound/soc/codecs/hdmi-codec.c | 8 +---
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/include/sound/hdmi-codec.h b/include/sound/hdmi-codec.h
index 9483c55f871b..89fc4cce5785 100644
--- a/include/sound/hdmi-codec.h
+++ b/include/sound/hdmi-codec.h
@@ -104,6 +104,7 @@ struct hdmi_codec_pdata {
uint i2s:1;
uint spdif:1;
int max_i2s_channels;
+ u64 formats;
void *data;
};
diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
index 22ed0dc88f0a..a7b4d6757ff1 100644
--- a/sound/soc/codecs/hdmi-codec.c
+++ b/sound/soc/codecs/hdmi-codec.c
@@ -653,9 +653,8 @@ static const struct snd_soc_dai_ops hdmi_dai_ops = {
* This list is only for formats allowed on the I2S bus. So there is
* some formats listed that are not supported by HDMI interface. For
* instance allowing the 32-bit formats enables 24-precision with CPU
- * DAIs that do not support 24-bit formats. If the extra formats cause
- * problems, we should add the video side driver an option to disable
- * them.
+ * DAIs that do not support 24-bit formats. Driver can either use this
+ * list or specify supported formats in formats field of hdmi_codec_pdata.
*/
#define I2S_FORMATS(SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE |\
SNDRV_PCM_FMTBIT_S20_3LE | SNDRV_PCM_FMTBIT_S20_3BE |\
@@ -780,6 +779,9 @@ static int hdmi_codec_probe(struct platform_device *pdev)
hcp->daidrv[i] = hdmi_i2s_dai;
hcp->daidrv[i].playback.channels_max =
hcd->max_i2s_channels;
+
+ if (hcd->formats)
+ hcp->daidrv[i].playback.formats = hcd->formats;
i++;
}
--
2.11.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/2] drm/bridge: adv7511: restrict audio sample sizes
From: Srinivas Kandagatla
ADV7533 only supports audio samples word width from 16-24 bits.
This patch restricts the audio sample sizes to the supported ones,
so that sound card does not report wrong list of supported hwparms.
Without this patch aplay would fail when playing a 32 bit audio.
Signed-off-by: Srinivas Kandagatla
---
drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
index 67469c26bae8..d01d0aa0eef7 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
@@ -214,6 +214,8 @@ static struct hdmi_codec_pdata codec_data = {
.ops = &adv7511_codec_ops,
.max_i2s_channels = 2,
.i2s = 1,
+ .formats = (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE |
+ SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S24_BE),
};
int adv7511_audio_init(struct device *dev, struct adv7511 *adv7511)
--
2.11.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] ASoC: hdmi-codec: Allow drivers to restrict sample sizes.
On 27/07/17 11:34, Jyri Sarha wrote:
On 07/17/17 17:02, srinivas.kandaga...@linaro.org wrote:
From: Srinivas Kandagatla
Currently hdmi client drivers does have means to limit the
sample sizes that it can only support. Having formats parameter
option would solve this.
This issue was noticed on DB410c board when adv7511 hdmi codec driver
failed to play a 32 bits audio samples, as it does not support them.
Signed-off-by: Srinivas Kandagatla
I left this feature out because I did not need it. All the HDMI encoders
I have used are able to take all the i2s formats I am able to send.
You should probably also update the comment above I2S_FORMATS
definition. Othewise:
Make sense, I will update it and send a v2 patch.
Reviewed-by: Jyri Sarha
thanks
--srini
---
include/sound/hdmi-codec.h| 1 +
sound/soc/codecs/hdmi-codec.c | 3 +++
2 files changed, 4 insertions(+)
diff --git a/include/sound/hdmi-codec.h b/include/sound/hdmi-codec.h
index 9483c55f871b..89fc4cce5785 100644
--- a/include/sound/hdmi-codec.h
+++ b/include/sound/hdmi-codec.h
@@ -104,6 +104,7 @@ struct hdmi_codec_pdata {
uint i2s:1;
uint spdif:1;
int max_i2s_channels;
+ u64 formats;
void *data;
};
diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
index 22ed0dc88f0a..1ece73f4be92 100644
--- a/sound/soc/codecs/hdmi-codec.c
+++ b/sound/soc/codecs/hdmi-codec.c
@@ -780,6 +780,9 @@ static int hdmi_codec_probe(struct platform_device *pdev)
hcp->daidrv[i] = hdmi_i2s_dai;
hcp->daidrv[i].playback.channels_max =
hcd->max_i2s_channels;
+
+ if (hcd->formats)
+ hcp->daidrv[i].playback.formats = hcd->formats;
i++;
}
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/bridge: adv7511: restrict audio sample sizes
From: Srinivas Kandagatla
ADV7533 only supports audio samples word width from 16-24 bits.
This patch restricts the audio sample sizes to the supported ones,
so that sound card does not report wrong list of supported hwparms.
Without this patch aplay would fail when playing a 32 bit audio.
Signed-off-by: Srinivas Kandagatla
---
drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
index 67469c26bae8..d01d0aa0eef7 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
@@ -214,6 +214,8 @@ static struct hdmi_codec_pdata codec_data = {
.ops = &adv7511_codec_ops,
.max_i2s_channels = 2,
.i2s = 1,
+ .formats = (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE |
+ SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S24_BE),
};
int adv7511_audio_init(struct device *dev, struct adv7511 *adv7511)
--
2.11.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] ASoC: hdmi-codec: Allow drivers to restrict sample sizes.
From: Srinivas Kandagatla
Currently hdmi client drivers does have means to limit the
sample sizes that it can only support. Having formats parameter
option would solve this.
This issue was noticed on DB410c board when adv7511 hdmi codec driver
failed to play a 32 bits audio samples, as it does not support them.
Signed-off-by: Srinivas Kandagatla
---
include/sound/hdmi-codec.h| 1 +
sound/soc/codecs/hdmi-codec.c | 3 +++
2 files changed, 4 insertions(+)
diff --git a/include/sound/hdmi-codec.h b/include/sound/hdmi-codec.h
index 9483c55f871b..89fc4cce5785 100644
--- a/include/sound/hdmi-codec.h
+++ b/include/sound/hdmi-codec.h
@@ -104,6 +104,7 @@ struct hdmi_codec_pdata {
uint i2s:1;
uint spdif:1;
int max_i2s_channels;
+ u64 formats;
void *data;
};
diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
index 22ed0dc88f0a..1ece73f4be92 100644
--- a/sound/soc/codecs/hdmi-codec.c
+++ b/sound/soc/codecs/hdmi-codec.c
@@ -780,6 +780,9 @@ static int hdmi_codec_probe(struct platform_device *pdev)
hcp->daidrv[i] = hdmi_i2s_dai;
hcp->daidrv[i].playback.channels_max =
hcd->max_i2s_channels;
+
+ if (hcd->formats)
+ hcp->daidrv[i].playback.formats = hcd->formats;
i++;
}
--
2.11.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/2] ASoC: hdmi-codec: Allow driver to restrict sample size
From: Srinivas Kandagatla This patch adds option to allow hdmi-codec drivers to restrict the audio sample size based on the values that are suppored in hardware. First patch is adding that formats option to hdmi_codec_pdata and second patch is a fix in adv7511 codec driver which only supports 16 and 24 bit samples. Srinivas Kandagatla (2): ASoC: hdmi-codec: Allow drivers to restrict sample sizes. drm/bridge: adv7511: restrict audio sample sizes drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 2 ++ include/sound/hdmi-codec.h | 1 + sound/soc/codecs/hdmi-codec.c | 3 +++ 3 files changed, 6 insertions(+) -- 2.11.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC][PATCH 2/4] drm/bridge: adv7511: Add Audio support.
Few Minor comments below..
On 23/08/16 06:50, John Stultz wrote:
> This patch adds support to Audio for both adv7511 and adv7533
> bridge chips.
>
> This patch was originally from [1] by Lars-Peter Clausen
> and was adapted by Archit Taneja and
> Srinivas Kandagatla .
>
> Then I heavily reworked it to use the hdmi-codec driver. So credit
> to them, but blame to me.
>
> [1]
> https://github.com/analogdevicesinc/linux/blob/xcomm_zynq/drivers/gpu/drm/i2c/adv7511_audio.c
>
> Cc: David Airlie
> Cc: Archit Taneja
> Cc: Laurent Pinchart
> Cc: Wolfram Sang
> Cc: Srinivas Kandagatla
> Cc: "Ville Syrjälä"
> Cc: Boris Brezillon
> Cc: Andy Green
> Cc: Dave Long
> Cc: Guodong Xu
> Cc: Zhangfei Gao
> Cc: Mark Brown
> Cc: Lars-Peter Clausen
> Cc: Jose Abreu
> Cc: dri-devel at lists.freedesktop.org
> Signed-off-by: John Stultz
> ---
> drivers/gpu/drm/bridge/adv7511/Kconfig | 1 +
> drivers/gpu/drm/bridge/adv7511/Makefile| 2 +-
> drivers/gpu/drm/bridge/adv7511/adv7511.h | 4 +
> drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 208
> +
> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 5 +
> 5 files changed, 219 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/Kconfig
> b/drivers/gpu/drm/bridge/adv7511/Kconfig
> index d2b0499..e3975e9 100644
> --- a/drivers/gpu/drm/bridge/adv7511/Kconfig
> +++ b/drivers/gpu/drm/bridge/adv7511/Kconfig
> @@ -3,6 +3,7 @@ config DRM_I2C_ADV7511
> depends on OF
> select DRM_KMS_HELPER
> select REGMAP_I2C
> + select SND_SOC_HDMI_CODEC if SND_SOC
> help
> Support for the Analog Device ADV7511(W) and ADV7513 HDMI encoders.
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/Makefile
> b/drivers/gpu/drm/bridge/adv7511/Makefile
> index 9019327..ad7245d 100644
> --- a/drivers/gpu/drm/bridge/adv7511/Makefile
> +++ b/drivers/gpu/drm/bridge/adv7511/Makefile
> @@ -1,3 +1,3 @@
> -adv7511-y := adv7511_drv.o
> +adv7511-y := adv7511_drv.o adv7511_audio.o
> adv7511-$(CONFIG_DRM_I2C_ADV7533) += adv7533.o
> obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511.o
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> index c7002a0..594ee0e1 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -23,6 +23,8 @@ struct adv7511;
>
> int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet);
> int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet);
> +int adv7511_audio_init(struct device *dev);
> +void adv7511_audio_exit(struct device *dev);
>
> #define ADV7511_REG_CHIP_REVISION0x00
> #define ADV7511_REG_N0 0x01
> @@ -317,6 +319,8 @@ struct adv7511 {
> struct drm_display_mode curr_mode;
>
> unsigned int f_tmds;
> + unsigned int f_audio;
> + unsigned int audio_source;
>
> unsigned int current_edid_segment;
> uint8_t edid_buf[256];
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
> new file mode 100644
> index 000..5aa9809
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
> @@ -0,0 +1,208 @@
> +/*
> + * Analog Devices ADV7511 HDMI transmitter driver
> + *
> + * Copyright 2012 Analog Devices Inc.
> + * Copyright (c) 2016, Linaro Limited
> + *
> + * Licensed under the GPL-2.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
Looks like some of these includes are really not needed.
> +
> +#include "adv7511.h"
> +
> +static void adv7511_calc_cts_n(unsigned int f_tmds, unsigned int fs,
> +unsigned int *cts, unsigned int *n)
> +{
> + switch (fs) {
> + case 32000:
> + *n = 4096;
> + break;
> + case 44100:
> + *n = 6272;
> + break;
> + case 48000:
> + *n = 6144;
> + break;
> + }
> +
> + *cts = ((f_tmds * *n) / (128 * fs)) * 1000;
> +}
> +
> +static int adv7511_update_cts_n(struct adv7511 *adv7511)
> +{
> + unsigned int cts = 0;
> + unsigned int n = 0;
> +
> + adv7511_calc_cts_n(adv7511->f_tmds, adv7511->f_audio, &cts, &n);
> +
> + regmap_write(adv7511->regmap, ADV75
[RFC][PATCH 0/4] Audio support for adv7511 hdmi bridge
Hi John, On 23/08/16 06:50, John Stultz wrote: > This is an initial swing at getting the adv7511 hdmi bridge > audio support reviewed. > > I've taken the core audio work done by Lars-Peter Clausen, and > adapted by Srinivas Kandagatla and Archit Taneja, and tried to > rework it to use the hdmi-codec sound driver. > > This patchset, along with the i2s driver and dts changes allows > HDMI audio to work on the HiKey board. > > I'd really appreciate any thoughts or feedback. Thanks for putting up these patches together, I tested them on DB410c on top of v4.8-rc3 with a dt patch(https://git.linaro.org/people/srinivas.kandagatla/linux.git/commit/ac06c0e91b1a0ac83a0d2946e593fcdd73836896) and it works great. Tested-by: Srinivas Kandagatla thanks, srini > > thanks > -john > > Cc: David Airlie > Cc: Archit Taneja > Cc: Laurent Pinchart > Cc: Wolfram Sang > Cc: Srinivas Kandagatla > Cc: "Ville Syrjälä" > Cc: Boris Brezillon > Cc: Andy Green > Cc: Dave Long > Cc: Guodong Xu > Cc: Zhangfei Gao > Cc: Mark Brown > Cc: Lars-Peter Clausen > Cc: Jose Abreu > Cc: dri-devel at lists.freedesktop.org > Andy Green (1): > drm/bridge: adv7511: Initialize audio packet on adv7533 > > Archit Taneja (1): > drm/bridge: adv7511: Move the common data structures to header file > > John Stultz (1): > drm/bridge: adv7511: Add Audio support. > > Srinivas Kandagatla (1): > drm/bridge: adv7511: Enable the audio data and clock pads on adv7533 > > drivers/gpu/drm/bridge/adv7511/Kconfig | 1 + > drivers/gpu/drm/bridge/adv7511/Makefile| 2 +- > drivers/gpu/drm/bridge/adv7511/adv7511.h | 12 ++ > drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 208 > + > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 9 +- > drivers/gpu/drm/bridge/adv7511/adv7533.c | 23 +++ > 6 files changed, 252 insertions(+), 3 deletions(-) > create mode 100644 drivers/gpu/drm/bridge/adv7511/adv7511_audio.c >
[PATCH v2] drm: msm: Add ASoC generic hdmi audio codec support.
This patch adds support to generic audio codec via
ASoC hdmi-codec infrastucture which is merged recently.
Signed-off-by: Srinivas Kandagatla
---
Changes since v1:
-Fixed typo for 44.1Kz case spotted by Jyri Sarha
drivers/gpu/drm/msm/Kconfig | 1 +
drivers/gpu/drm/msm/hdmi/hdmi.c | 120 +++-
drivers/gpu/drm/msm/hdmi/hdmi.h | 14 +
3 files changed, 134 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index 167a497..7c7a031 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -10,6 +10,7 @@ config DRM_MSM
select SHMEM
select TMPFS
select QCOM_SCM
+ select SND_SOC_HDMI_CODEC if SND_SOC
default y
help
DRM/KMS driver for MSM/snapdragon.
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 51b9ea5..aada355 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -19,6 +19,7 @@
#include
#include
+#include
#include "hdmi.h"
void msm_hdmi_set_mode(struct hdmi *hdmi, bool power_on)
@@ -434,6 +435,114 @@ static int msm_hdmi_get_gpio(struct device_node *of_node,
const char *name)
return gpio;
}
+/*
+ * HDMI audio codec callbacks
+ */
+static int msm_hdmi_audio_hw_params(struct device *dev,
+ struct hdmi_codec_daifmt *daifmt,
+ struct hdmi_codec_params *params)
+{
+ struct hdmi *hdmi = dev_get_drvdata(dev);
+ unsigned int chan;
+ unsigned int channel_allocation = 0;
+ unsigned int rate;
+ unsigned int level_shift = 0; /* 0dB */
+ bool down_mix = false;
+
+ dev_dbg(dev, "%u Hz, %d bit, %d channels\n", params->sample_rate,
+params->sample_width, params->cea.channels);
+
+ switch (params->cea.channels) {
+ case 2:
+ /* FR and FL speakers */
+ channel_allocation = 0;
+ chan = MSM_HDMI_AUDIO_CHANNEL_2;
+ break;
+ case 4:
+ /* FC, LFE, FR and FL speakers */
+ channel_allocation = 0x3;
+ chan = MSM_HDMI_AUDIO_CHANNEL_4;
+ break;
+ case 6:
+ /* RR, RL, FC, LFE, FR and FL speakers */
+ channel_allocation = 0x0B;
+ chan = MSM_HDMI_AUDIO_CHANNEL_6;
+ break;
+ case 8:
+ /* FRC, FLC, RR, RL, FC, LFE, FR and FL speakers */
+ channel_allocation = 0x1F;
+ chan = MSM_HDMI_AUDIO_CHANNEL_8;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ switch (params->sample_rate) {
+ case 32000:
+ rate = HDMI_SAMPLE_RATE_32KHZ;
+ break;
+ case 44100:
+ rate = HDMI_SAMPLE_RATE_44_1KHZ;
+ break;
+ case 48000:
+ rate = HDMI_SAMPLE_RATE_48KHZ;
+ break;
+ case 88200:
+ rate = HDMI_SAMPLE_RATE_88_2KHZ;
+ break;
+ case 96000:
+ rate = HDMI_SAMPLE_RATE_96KHZ;
+ break;
+ case 176400:
+ rate = HDMI_SAMPLE_RATE_176_4KHZ;
+ break;
+ case 192000:
+ rate = HDMI_SAMPLE_RATE_192KHZ;
+ break;
+ default:
+ dev_err(dev, "rate[%d] not supported!\n",
+ params->sample_rate);
+ return -EINVAL;
+ }
+
+ msm_hdmi_audio_set_sample_rate(hdmi, rate);
+ msm_hdmi_audio_info_setup(hdmi, 1, chan, channel_allocation,
+ level_shift, down_mix);
+
+ return 0;
+}
+
+static void msm_hdmi_audio_shutdown(struct device *dev)
+{
+ struct hdmi *hdmi = dev_get_drvdata(dev);
+
+ msm_hdmi_audio_info_setup(hdmi, 0, 0, 0, 0, 0);
+}
+
+static const struct hdmi_codec_ops msm_hdmi_audio_codec_ops = {
+ .hw_params = msm_hdmi_audio_hw_params,
+ .audio_shutdown = msm_hdmi_audio_shutdown,
+};
+
+static struct hdmi_codec_pdata codec_data = {
+ .ops = &msm_hdmi_audio_codec_ops,
+ .max_i2s_channels = 8,
+ .i2s = 1,
+};
+
+static int msm_hdmi_register_audio_driver(struct hdmi *hdmi, struct device
*dev)
+{
+ hdmi->audio_pdev = platform_device_register_data(dev,
+HDMI_CODEC_DRV_NAME,
+PLATFORM_DEVID_AUTO,
+&codec_data,
+sizeof(codec_data));
+ if (IS_ERR(hdmi->audio_pdev))
+ return PTR_ERR(hdmi->audio_pdev);
+
+ return 0;
+}
+
static int msm_hdmi_bind(struct device *dev, struct device *master, void *data)
{
struct drm_devi
[RFC PATCH] drm: msm: Add ASoC generic hdmi audio codec support.
On 06/06/16 13:23, Jyri Sarha wrote: > On 06/03/16 16:56, Srinivas Kandagatla wrote: >> >This patch adds support to generic audio codec via >> >ASoC hdmi-codec infrastucture which is merged recently. >> > > I know nothing about msm HW, but from the hdmi-codec point of view this > looks like a correct usage. However, the hdmi-codec could probably do > more to connect the hdmi audio infoframe's channel allocation field and > ALSA's channel mapping API together. Yes, that would be nice to get the channel allocation into hdmi-codec. > > There looks to be a bug in selecting 44100 sample rate bellow. > Thanks for spotting this, yes this looks like a typo, I will fix this in v2. Thanks, srini > BR, > Jyri > >> >Signed-off-by: Srinivas Kandagatla >> >--- >> > drivers/gpu/drm/msm/Kconfig | 1 + >> > drivers/gpu/drm/msm/hdmi/hdmi.c | 120 >> > +++- >> > drivers/gpu/drm/msm/hdmi/hdmi.h | 14 + >> > 3 files changed, 134 insertions(+), 1 deletion(-) >> >> >diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c >> >b/drivers/gpu/drm/msm/hdmi/hdmi.c >> >index 51b9ea5..3281496 100644 >> >--- a/drivers/gpu/drm/msm/hdmi/hdmi.c >> >+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c >> >@@ -19,6 +19,7 @@ >> > #include >> > #include >> > >> >+#include >> > #include "hdmi.h" >> > >> > void msm_hdmi_set_mode(struct hdmi *hdmi, bool power_on) >> >@@ -434,6 +435,114 @@ static int msm_hdmi_get_gpio(struct device_node >> >*of_node, const char *name) >> >return gpio; ... >> >+ case 32000: >> >+ rate = HDMI_SAMPLE_RATE_32KHZ; >> >+ break; >> >+ case 44100: >> >+ rate = HDMI_SAMPLE_RATE_48KHZ; >> >+ break; > This looks like a bug... >
[RFC PATCH] drm: msm: Add ASoC generic hdmi audio codec support.
This patch adds support to generic audio codec via
ASoC hdmi-codec infrastucture which is merged recently.
Signed-off-by: Srinivas Kandagatla
---
drivers/gpu/drm/msm/Kconfig | 1 +
drivers/gpu/drm/msm/hdmi/hdmi.c | 120 +++-
drivers/gpu/drm/msm/hdmi/hdmi.h | 14 +
3 files changed, 134 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index 167a497..7c7a031 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -10,6 +10,7 @@ config DRM_MSM
select SHMEM
select TMPFS
select QCOM_SCM
+ select SND_SOC_HDMI_CODEC if SND_SOC
default y
help
DRM/KMS driver for MSM/snapdragon.
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 51b9ea5..3281496 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -19,6 +19,7 @@
#include
#include
+#include
#include "hdmi.h"
void msm_hdmi_set_mode(struct hdmi *hdmi, bool power_on)
@@ -434,6 +435,114 @@ static int msm_hdmi_get_gpio(struct device_node *of_node,
const char *name)
return gpio;
}
+/*
+ * HDMI audio codec callbacks
+ */
+static int msm_hdmi_audio_hw_params(struct device *dev,
+ struct hdmi_codec_daifmt *daifmt,
+ struct hdmi_codec_params *params)
+{
+ struct hdmi *hdmi = dev_get_drvdata(dev);
+ unsigned int chan;
+ unsigned int channel_allocation = 0;
+ unsigned int rate;
+ unsigned int level_shift = 0; /* 0dB */
+ bool down_mix = false;
+
+ dev_dbg(dev, "%u Hz, %d bit, %d channels\n", params->sample_rate,
+params->sample_width, params->cea.channels);
+
+ switch (params->cea.channels) {
+ case 2:
+ /* FR and FL speakers */
+ channel_allocation = 0;
+ chan = MSM_HDMI_AUDIO_CHANNEL_2;
+ break;
+ case 4:
+ /* FC, LFE, FR and FL speakers */
+ channel_allocation = 0x3;
+ chan = MSM_HDMI_AUDIO_CHANNEL_4;
+ break;
+ case 6:
+ /* RR, RL, FC, LFE, FR and FL speakers */
+ channel_allocation = 0x0B;
+ chan = MSM_HDMI_AUDIO_CHANNEL_6;
+ break;
+ case 8:
+ /* FRC, FLC, RR, RL, FC, LFE, FR and FL speakers */
+ channel_allocation = 0x1F;
+ chan = MSM_HDMI_AUDIO_CHANNEL_8;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ switch (params->sample_rate) {
+ case 32000:
+ rate = HDMI_SAMPLE_RATE_32KHZ;
+ break;
+ case 44100:
+ rate = HDMI_SAMPLE_RATE_48KHZ;
+ break;
+ case 48000:
+ rate = HDMI_SAMPLE_RATE_48KHZ;
+ break;
+ case 88200:
+ rate = HDMI_SAMPLE_RATE_88_2KHZ;
+ break;
+ case 96000:
+ rate = HDMI_SAMPLE_RATE_96KHZ;
+ break;
+ case 176400:
+ rate = HDMI_SAMPLE_RATE_176_4KHZ;
+ break;
+ case 192000:
+ rate = HDMI_SAMPLE_RATE_192KHZ;
+ break;
+ default:
+ dev_err(dev, "rate[%d] not supported!\n",
+ params->sample_rate);
+ return -EINVAL;
+ }
+
+ msm_hdmi_audio_set_sample_rate(hdmi, rate);
+ msm_hdmi_audio_info_setup(hdmi, 1, chan, channel_allocation,
+ level_shift, down_mix);
+
+ return 0;
+}
+
+static void msm_hdmi_audio_shutdown(struct device *dev)
+{
+ struct hdmi *hdmi = dev_get_drvdata(dev);
+
+ msm_hdmi_audio_info_setup(hdmi, 0, 0, 0, 0, 0);
+}
+
+static const struct hdmi_codec_ops msm_hdmi_audio_codec_ops = {
+ .hw_params = msm_hdmi_audio_hw_params,
+ .audio_shutdown = msm_hdmi_audio_shutdown,
+};
+
+static struct hdmi_codec_pdata codec_data = {
+ .ops = &msm_hdmi_audio_codec_ops,
+ .max_i2s_channels = 8,
+ .i2s = 1,
+};
+
+static int msm_hdmi_register_audio_driver(struct hdmi *hdmi, struct device
*dev)
+{
+ hdmi->audio_pdev = platform_device_register_data(dev,
+HDMI_CODEC_DRV_NAME,
+PLATFORM_DEVID_AUTO,
+&codec_data,
+sizeof(codec_data));
+ if (IS_ERR(hdmi->audio_pdev))
+ return PTR_ERR(hdmi->audio_pdev);
+
+ return 0;
+}
+
static int msm_hdmi_bind(struct device *dev, struct device *master, void *data)
{
struct drm_device *drm = dev_get_drvdata(master);
@@ -441,7 +550,7 @@ static int msm_hdmi_bind(struc
[PATCH RFC 1/5] drm/edid: Add support to get edid early
On 17/08/15 08:52, Jani Nikula wrote:
> On Fri, 14 Aug 2015, Srinivas Kandagatla
> wrote:
>> This patch adds support to get edid way early before the connector is
>> created, this is mainly used for panel drivers to auto-probe the panel
>> based on the vendor and product id from EDID.
>>
>> Signed-off-by: Srinivas Kandagatla
>> ---
>> drivers/gpu/drm/drm_edid.c | 8
>> include/drm/drm_crtc.h | 1 +
>> 2 files changed, 9 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 7087da3..30359cd 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -1388,6 +1388,14 @@ struct edid *drm_get_edid(struct drm_connector
>> *connector,
>> }
>> EXPORT_SYMBOL(drm_get_edid);
>>
>> +struct edid *drm_get_edid_early(struct i2c_adapter *adapter)
>> +{
>> +struct drm_connector dummy_connector;
>> +
>> +return drm_get_edid(&dummy_connector, adapter);
>
> This will oops the kernel on bad EDID.
>
Thanks for quick review,
Yes, you are right it would blow up on dev_warn(connector->dev->dev, ...
May we can fix this if we are happy to take this approach of getting
edid early.
--srini
> BR,
> Jani.
>
>
>> +}
>> +EXPORT_SYMBOL(drm_get_edid_early);
>> +
>> /**
>>* drm_edid_duplicate - duplicate an EDID and the extensions
>>* @edid: EDID to duplicate
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> index 57ca8cc..35d8763 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -1330,6 +1330,7 @@ extern void drm_reinit_primary_mode_group(struct
>> drm_device *dev);
>> extern bool drm_probe_ddc(struct i2c_adapter *adapter);
>> extern struct edid *drm_get_edid(struct drm_connector *connector,
>> struct i2c_adapter *adapter);
>> +extern struct edid *drm_get_edid_early(struct i2c_adapter *adapter);
>> extern struct edid *drm_edid_duplicate(const struct edid *edid);
>> extern int drm_add_edid_modes(struct drm_connector *connector, struct edid
>> *edid);
>> extern void drm_mode_config_init(struct drm_device *dev);
>> --
>> 1.9.1
>>
>> ___
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
[PATCH RFC 5/5] drm/msm: mdp4 lvds: Check the panel node in detect_panel()
This patch checks if the panel node is disabled in DT or not, this would
let us return proper error code so that the driver could stop panel
specific intialization.
Signed-off-by: Srinivas Kandagatla
---
drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
index ad02b8b..167f402 100644
--- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
+++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
@@ -262,6 +262,11 @@ static struct drm_panel *detect_panel(struct drm_device
*dev)
of_node_put(endpoint);
+ if (!of_device_is_available(panel_node)) {
+ dev_err(dev->dev, "panel is not enabled in DT\n");
+ return ERR_PTR(-ENODEV);
+ }
+
panel = of_drm_find_panel(panel_node);
if (!panel) {
of_node_put(panel_node);
--
1.9.1
[PATCH RFC 4/5] drm/msm: mdp4 lvds: continue if the panel is not connected
Two issues:
1> Intializing panel specific bits without actual panel presence.
2> Bailing out if the detect_panel() return -ENODEV.
With the existing code if detect_panel() returns an error code the
driver would bail out without doing anything, However it could continue
intializing hdmi related things. This patch adds two things.
1> moves the panel specific intialization only if the panel is detected
2> let the driver continue with hdmi intialization if detect_panel()
return -ENODEV.
Signed-off-by: Srinivas Kandagatla
---
drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 72 +++--
1 file changed, 41 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
index 601dcc0..ad02b8b 100644
--- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
+++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
@@ -313,45 +313,55 @@ static int modeset_init(struct mdp4_kms *mdp4_kms)
if (IS_ERR(panel)) {
ret = PTR_ERR(panel);
dev_err(dev->dev, "failed to detect LVDS panel: %d\n", ret);
- goto fail;
- }
+ /**
+* Only fail if there is panel but not ready yet
+* continue with other stuff if there is no panel connected.
+*/
+ if (ret == -EPROBE_DEFER)
+ goto fail;
+ } else {
+ plane = mdp4_plane_init(dev, RGB2, true);
+ if (IS_ERR(plane)) {
+ dev_err(dev->dev,
+ "failed to construct plane for RGB2\n");
+ ret = PTR_ERR(plane);
+ goto fail;
+ }
- plane = mdp4_plane_init(dev, RGB2, true);
- if (IS_ERR(plane)) {
- dev_err(dev->dev, "failed to construct plane for RGB2\n");
- ret = PTR_ERR(plane);
- goto fail;
- }
+ crtc = mdp4_crtc_init(dev, plane, priv->num_crtcs, 0, DMA_P);
+ if (IS_ERR(crtc)) {
+ dev_err(dev->dev,
+ "failed to construct crtc for DMA_P\n");
+ ret = PTR_ERR(crtc);
+ goto fail;
+ }
- crtc = mdp4_crtc_init(dev, plane, priv->num_crtcs, 0, DMA_P);
- if (IS_ERR(crtc)) {
- dev_err(dev->dev, "failed to construct crtc for DMA_P\n");
- ret = PTR_ERR(crtc);
- goto fail;
- }
+ encoder = mdp4_lcdc_encoder_init(dev, panel);
+ if (IS_ERR(encoder)) {
+ dev_err(dev->dev,
+ "failed to construct LCDC encoder\n");
+ ret = PTR_ERR(encoder);
+ goto fail;
+ }
- encoder = mdp4_lcdc_encoder_init(dev, panel);
- if (IS_ERR(encoder)) {
- dev_err(dev->dev, "failed to construct LCDC encoder\n");
- ret = PTR_ERR(encoder);
- goto fail;
- }
+ /* LCDC can be hooked to DMA_P: */
+ encoder->possible_crtcs = 1 << priv->num_crtcs;
- /* LCDC can be hooked to DMA_P: */
- encoder->possible_crtcs = 1 << priv->num_crtcs;
+ priv->crtcs[priv->num_crtcs++] = crtc;
+ priv->encoders[priv->num_encoders++] = encoder;
- priv->crtcs[priv->num_crtcs++] = crtc;
- priv->encoders[priv->num_encoders++] = encoder;
+ connector = mdp4_lvds_connector_init(dev, panel, encoder);
+ if (IS_ERR(connector)) {
+ ret = PTR_ERR(connector);
+ dev_err(dev->dev,
+ "failed to initialize LVDS connector: %d\n",
+ ret);
+ goto fail;
+ }
- connector = mdp4_lvds_connector_init(dev, panel, encoder);
- if (IS_ERR(connector)) {
- ret = PTR_ERR(connector);
- dev_err(dev->dev, "failed to initialize LVDS connector: %d\n",
ret);
- goto fail;
+ priv->connectors[priv->num_connectors++] = connector;
}
- priv->connectors[priv->num_connectors++] = connector;
-
/*
* Setup DTV/HDMI path: RGB1 -> DMA_E -> DTV -> HDMI:
*/
--
1.9.1
[PATCH RFC 3/5] drm/panel: simple-panel: Add panel picker support.
This patch adds panel picker support to simple-panel.
The idea of panel picker is to select the correct panel timings if it
supports probing edid via DDC bus, edid contains manufacture ID
and Manufacturer product code, so it can match against the panel_picker
entries to get the correct panel timings.
>From DT point of view the panel picker uses generic compatible string
"panel-simple", keeping the panel specific compatible strings still
supported.
Panels can be static entry in the DT, but practically development boards
like IFC6410 where developers can connect any LVDS panel which makes it
difficult to maintian the dt support for those panels in dts file.
With this dynamic probing via panel picker makes it easy to support such
use-cases.
This patch also adds panel presence detection based, if there is no
panel detected or panel picker could not find the panel then the driver
would mark the panel DT node as disabled so that the drm driver would
be able to take right decision based on that panel node status.
Signed-off-by: Srinivas Kandagatla
---
drivers/gpu/drm/panel/panel-simple.c | 83 +++-
1 file changed, 81 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-simple.c
b/drivers/gpu/drm/panel/panel-simple.c
index f94201b..6c503b2 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -32,6 +32,7 @@
#include
#include
#include
+#include
#include
#include
@@ -70,6 +71,18 @@ struct panel_desc {
u32 bus_format;
};
+#define PANEL_PICKER_ENTRY(vend, pid, pdesc) \
+ .vendor = vend, \
+ .product_id = (pid), \
+ .data = (pdesc)
+
+/* Panel picker entry with vendor and product id */
+struct panel_picker_entry {
+ char vendor[4]; /* Vendor string */
+ int product_id; /* product id field */
+ const struct panel_desc *data;
+};
+
struct panel_simple {
struct drm_panel base;
bool prepared;
@@ -84,6 +97,8 @@ struct panel_simple {
struct gpio_desc *enable_gpio;
};
+static const struct panel_desc *panel_picker_find_panel(struct edid *edid);
+
static inline struct panel_simple *to_panel_simple(struct drm_panel *panel)
{
return container_of(panel, struct panel_simple, base);
@@ -276,11 +291,28 @@ static const struct drm_panel_funcs panel_simple_funcs = {
.get_timings = panel_simple_get_timings,
};
+static void __init simple_panel_node_disable(struct device_node *node)
+{
+ struct property *prop;
+
+ prop = kzalloc(sizeof(*prop), GFP_KERNEL);
+ if (!prop)
+ return;
+
+ prop->name = "status";
+ prop->value = "disabled";
+ prop->length = strlen((char *)prop->value)+1;
+
+ of_update_property(node, prop);
+}
+
+
static int panel_simple_probe(struct device *dev, const struct panel_desc
*desc)
{
struct device_node *backlight, *ddc;
struct panel_simple *panel;
int err;
+ struct edid *edid;
panel = devm_kzalloc(dev, sizeof(*panel), GFP_KERNEL);
if (!panel)
@@ -288,7 +320,6 @@ static int panel_simple_probe(struct device *dev, const
struct panel_desc *desc)
panel->enabled = false;
panel->prepared = false;
- panel->desc = desc;
panel->supply = devm_regulator_get(dev, "power");
if (IS_ERR(panel->supply))
@@ -316,7 +347,25 @@ static int panel_simple_probe(struct device *dev, const
struct panel_desc *desc)
panel->ddc = of_find_i2c_adapter_by_node(ddc);
of_node_put(ddc);
- if (!panel->ddc) {
+ if (panel->ddc) {
+ /* detect panel presence */
+ if (!drm_probe_ddc(panel->ddc)) {
+ err = -ENODEV;
+ goto nodev;
+ }
+
+ /* get panel from edid */
+ if (of_device_is_compatible(dev->of_node,
+ "panel-simple")) {
+ edid = drm_get_edid_early(panel->ddc);
+ if (edid) {
+ desc = panel_picker_find_panel(edid);
+ } else {
+ err = -ENODEV;
+ goto nodev;
+ }
+ }
+ } else {
err = -EPROBE_DEFER;
goto free_backlight;
}
@@ -325,6 +374,7 @@ static int panel_simple_probe(struct device *dev, const
struct panel_desc *desc)
drm_panel_init(&panel->base);
panel->base.dev = dev;
panel->base.funcs = &panel_simple_funcs;
+ panel->desc = desc;
er
[PATCH RFC 2/5] drm/edid: export edid_vendor()
This patch exports edid_vendor() match function, so that other drivers
could use it for matching the vendor id.
Signed-off-by: Srinivas Kandagatla
---
drivers/gpu/drm/drm_edid.c | 4 ++--
include/drm/drm_crtc.h | 1 +
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 30359cd..e0376ad 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1417,7 +1417,7 @@ EXPORT_SYMBOL(drm_edid_duplicate);
*
* Returns true if @vendor is in @edid, false otherwise
*/
-static bool edid_vendor(struct edid *edid, char *vendor)
+bool edid_vendor(struct edid *edid, char *vendor)
{
char edid_vendor[3];
@@ -1428,7 +1428,7 @@ static bool edid_vendor(struct edid *edid, char *vendor)
return !strncmp(edid_vendor, vendor, 3);
}
-
+EXPORT_SYMBOL(edid_vendor);
/**
* edid_get_quirks - return quirk flags for a given EDID
* @edid: EDID to process
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 35d8763..bb6dc61 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1332,6 +1332,7 @@ extern struct edid *drm_get_edid(struct drm_connector
*connector,
struct i2c_adapter *adapter);
extern struct edid *drm_get_edid_early(struct i2c_adapter *adapter);
extern struct edid *drm_edid_duplicate(const struct edid *edid);
+extern bool edid_vendor(struct edid *edid, char *vendor);
extern int drm_add_edid_modes(struct drm_connector *connector, struct edid
*edid);
extern void drm_mode_config_init(struct drm_device *dev);
extern void drm_mode_config_reset(struct drm_device *dev);
--
1.9.1
[PATCH RFC 1/5] drm/edid: Add support to get edid early
This patch adds support to get edid way early before the connector is
created, this is mainly used for panel drivers to auto-probe the panel
based on the vendor and product id from EDID.
Signed-off-by: Srinivas Kandagatla
---
drivers/gpu/drm/drm_edid.c | 8
include/drm/drm_crtc.h | 1 +
2 files changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 7087da3..30359cd 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1388,6 +1388,14 @@ struct edid *drm_get_edid(struct drm_connector
*connector,
}
EXPORT_SYMBOL(drm_get_edid);
+struct edid *drm_get_edid_early(struct i2c_adapter *adapter)
+{
+ struct drm_connector dummy_connector;
+
+ return drm_get_edid(&dummy_connector, adapter);
+}
+EXPORT_SYMBOL(drm_get_edid_early);
+
/**
* drm_edid_duplicate - duplicate an EDID and the extensions
* @edid: EDID to duplicate
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 57ca8cc..35d8763 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1330,6 +1330,7 @@ extern void drm_reinit_primary_mode_group(struct
drm_device *dev);
extern bool drm_probe_ddc(struct i2c_adapter *adapter);
extern struct edid *drm_get_edid(struct drm_connector *connector,
struct i2c_adapter *adapter);
+extern struct edid *drm_get_edid_early(struct i2c_adapter *adapter);
extern struct edid *drm_edid_duplicate(const struct edid *edid);
extern int drm_add_edid_modes(struct drm_connector *connector, struct edid
*edid);
extern void drm_mode_config_init(struct drm_device *dev);
--
1.9.1
[PATCH RFC 0/5] drm/panel: simple: Add panel picker support.
Hi All, This patchset adds support to Panel Picker. The idea of panel picker is to pick a panel and its timing based on "Manufacturer ID" and "Manufacturer product code" which is available in the EDID. For now I added only one panel picker entry as a proof of concept, as we move on we could add more entires to the list. Reason why I started to look in to this side is - to have a DT for panel which could work with multiple panels. - to detect the panel presence. Simple panel driver supports more than 25 panels, having compatible string for each to get the panel descriptor. The problem with such approch is that if we connect a panel to a board whose compatible string is different to the one mentioned in DT, it would not work, eventhough the simple panel driver supports it. Maybe we are saying that if we replace a panel on your phone with different manufacturer it would need a software upgrade? :-) For example when I submitted a DT patch to add LVDS panel support to IFC6410, people started asking about two things, - How can it work with other panels, as they have different panels then mine. - Why is the hdmi display resolution changed with a LVDS panel DT entry? (this is due to drm driver does not know the panel presence) >From DT point of view the panel picker uses generic compatible string "panel-simple", keeping the panel specific compatible strings still supported. This patch also adds panel presence detection based, if there is no panel detected or panel picker could not find the panel then the driver would mark the panel DT node as disabled so that the drm driver would be able to take right decision based on that panel node status. There might be other solutions to this issue, but given the fact that most of boards we use we do not have access to bootloader code to modify the dt at runtime, so with that considered we came up with this solution which works on IFC6410 and would work on other boards too. I have also included the mdp4 fixes in this series too so that people can test it on IFC6410. These are WIP patches to get more inputs and comments. Thanks, srini Srinivas Kandagatla (5): drm/edid: Add support to get edid early drm/edid: export edid_vendor() drm/panel: simple-panel: Add panel picker support. drm/msm: mdp4 lvds: continue if the panel is not connected drm/msm: mdp4 lvds: Check the panel node in detect_panel() drivers/gpu/drm/drm_edid.c | 12 - drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 77 ++ drivers/gpu/drm/panel/panel-simple.c| 83 - include/drm/drm_crtc.h | 2 + 4 files changed, 139 insertions(+), 35 deletions(-) -- 1.9.1
[PATCH RFC 2/5] drm/msm/hdmi: make use of standard gpio properties.
On 10/08/15 14:26, Thierry Reding wrote:
> On Mon, Aug 10, 2015 at 02:15:18PM +0100, Srinivas Kandagatla wrote:
>>
>>
>> On 10/08/15 13:38, Thierry Reding wrote:
>>> On Mon, Aug 10, 2015 at 12:59:34PM +0100, Srinivas Kandagatla wrote:
>>>> This patch modifies the driver to support standard gpio properties along
>>>> with deprecated properties. This will help us upstream and cleanup the
>>>> non-standard properties over the time.
>>>>
>>>> Signed-off-by: Srinivas Kandagatla
>>>> ---
>>>> drivers/gpu/drm/msm/hdmi/hdmi.c | 35 +--
>>>> 1 file changed, 25 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c
>>>> b/drivers/gpu/drm/msm/hdmi/hdmi.c
>>>> index 8145362..e918889 100644
>>>> --- a/drivers/gpu/drm/msm/hdmi/hdmi.c
>>>> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
>>>> @@ -339,19 +339,34 @@ static const struct of_device_id dt_match[] = {
>>>> };
>>>>
>>>> #ifdef CONFIG_OF
>>>> +/* This code will be removed once we move to gpiod based calls */
>>>
>>> Why don't you do this now instead of duplicating what is essentially
>>> already implemented in gpiolib?
>>>
>> One of the thing that Rob asked in his comments
>> (http://www.spinics.net/lists/arm-kernel/msg437675.html) was to retain the
>> support for old devices, moving to gpiod ATM would break such devices as
>> they are still using legacy gpiolib and its naming.
>>
>>
>> If Rob is ok to drop gpio properties which does not have "-gpio" or "-gpios"
>> suffix then we can get rid of this function all together.
>
> If you make the switch to gpiod_*() APIs you'll get this for free.
> There's really no need for having a duplicate of what gpiod_get()
> already does for you.
>
Yes, you are right, my memory was over written on friday by looking at
of_find_gpio() for long :-)
AFAIK, gpiod_get would also request the gpios so we might want to remove
the additional gpio requests in the hdmi_connector.c too.
I will give that a try once again..
--srini
> Thierry
>
[PATCH RFC 4/5] drm/msm/hdmi: deprecate non standard clock-names
On 10/08/15 14:33, Thierry Reding wrote:
> On Mon, Aug 10, 2015 at 02:18:15PM +0100, Srinivas Kandagatla wrote:
>>
>>
>> On 10/08/15 13:49, Thierry Reding wrote:
>>> On Mon, Aug 10, 2015 at 12:59:49PM +0100, Srinivas Kandagatla wrote:
>>>> This patch updates the bindings to discourage the usage of non standard
>>>> clock names, this will help in projects focused on upstreaming.
>>>>
>>>> These deprecated properties are still supported but will be remove over
>>>> the time.
>>>>
>>>> Signed-off-by: Srinivas Kandagatla
>>>> ---
>>>> Documentation/devicetree/bindings/drm/msm/hdmi.txt | 16
>>>> 1 file changed, 8 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/drm/msm/hdmi.txt
>>>> b/Documentation/devicetree/bindings/drm/msm/hdmi.txt
>>>> index 6dc202e..6fbfdd8 100644
>>>> --- a/Documentation/devicetree/bindings/drm/msm/hdmi.txt
>>>> +++ b/Documentation/devicetree/bindings/drm/msm/hdmi.txt
>>>> @@ -12,16 +12,16 @@ Required properties:
>>>> - clocks: device clocks
>>>> - clock-names: Corresponding name for each entry in the clocks property.
>>>>for "qcom,hdmi-tx-8960" compatible names should be
>>>> - "core_clk"
>>>> - "master_iface_clk"
>>>> - "slave_iface_clk"
>>>> + "core_clk" is deprecated, use "core" instead
>>>> + "master_iface_clk" is deprecated, use "master_iface" instead
>>>> + "slave_iface_clk" is deprecated, use "slave_iface" instead
>>>>
>>>>for "qcom,hdmi-tx-8084" and "qcom,hdmi-tx-8074" compatible names should
>>>> be
>>>> - "extp_clk"
>>>> - "alt_iface_clk"
>>>> - "iface_clk"
>>>> - "core_clk"
>>>> - "mdp_core_clk"
>>>> + "extp_clk" is deprecated, use "extp" instead
>>>> + "alt_iface_clk" is deprecated, use "alt_iface" intstead
>>>> + "iface_clk" is deprecated, use "iface" instead
>>>> + "core_clk" is deprecated, use "core" instead
>>>> + "mdp_core_clk" is deprecated, use "mdp_core" instead
>>>
>>> Shouldn't there be a driver counterpart of this to accept the new names?
>> Driver changes are in this same series "[PATCH RFC 5/5] drm/msm/hdmi: remove
>> _clk suffix from clock names"(https://lkml.org/lkml/2015/8/10/453)
>
> I don't have that patch in my inbox. It looks to be doing things
> backwards (look up the deprecated name first). I think it should be:
>
> clk = devm_clk_get(dev, id);
> if (IS_ERR(clk)) {
If the clock controller is not ready yet, it would return EPROBE DEFER,
which gets dropped here, as a result the driver would not be probed again.
Probably both of the error codes needs be checked before returning.
> char clk_name[32];
>
> snprintf(clk_name, sizeof(clk_name), "%s_clk", id);
> clk = devm_clk_get(dev, clk_name);
> if (IS_ERR(clk))
> return clk;
> }
>
> Also note how I've dropped the ERR_CAST(), that's not useful here
> because you aren't actually casting, but simply returning clk.
>
> Thierry
>
[PATCH RFC 4/5] drm/msm/hdmi: deprecate non standard clock-names
On 10/08/15 13:49, Thierry Reding wrote: > On Mon, Aug 10, 2015 at 12:59:49PM +0100, Srinivas Kandagatla wrote: >> This patch updates the bindings to discourage the usage of non standard >> clock names, this will help in projects focused on upstreaming. >> >> These deprecated properties are still supported but will be remove over >> the time. >> >> Signed-off-by: Srinivas Kandagatla >> --- >> Documentation/devicetree/bindings/drm/msm/hdmi.txt | 16 >> 1 file changed, 8 insertions(+), 8 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/drm/msm/hdmi.txt >> b/Documentation/devicetree/bindings/drm/msm/hdmi.txt >> index 6dc202e..6fbfdd8 100644 >> --- a/Documentation/devicetree/bindings/drm/msm/hdmi.txt >> +++ b/Documentation/devicetree/bindings/drm/msm/hdmi.txt >> @@ -12,16 +12,16 @@ Required properties: >> - clocks: device clocks >> - clock-names: Corresponding name for each entry in the clocks property. >>for "qcom,hdmi-tx-8960" compatible names should be >> -"core_clk" >> -"master_iface_clk" >> -"slave_iface_clk" >> +"core_clk" is deprecated, use "core" instead >> +"master_iface_clk" is deprecated, use "master_iface" instead >> +"slave_iface_clk" is deprecated, use "slave_iface" instead >> >>for "qcom,hdmi-tx-8084" and "qcom,hdmi-tx-8074" compatible names should be >> -"extp_clk" >> -"alt_iface_clk" >> -"iface_clk" >> -"core_clk" >> -"mdp_core_clk" >> +"extp_clk" is deprecated, use "extp" instead >> +"alt_iface_clk" is deprecated, use "alt_iface" intstead >> +"iface_clk" is deprecated, use "iface" instead >> +"core_clk" is deprecated, use "core" instead >> +"mdp_core_clk" is deprecated, use "mdp_core" instead > > Shouldn't there be a driver counterpart of this to accept the new names? Driver changes are in this same series "[PATCH RFC 5/5] drm/msm/hdmi: remove _clk suffix from clock names"(https://lkml.org/lkml/2015/8/10/453) --srini > Otherwise people could be switching the DTS to the new value, but the > driver won't find the clocks it's looking for. > > Thierry >
[PATCH RFC 2/5] drm/msm/hdmi: make use of standard gpio properties.
On 10/08/15 13:38, Thierry Reding wrote:
> On Mon, Aug 10, 2015 at 12:59:34PM +0100, Srinivas Kandagatla wrote:
>> This patch modifies the driver to support standard gpio properties along
>> with deprecated properties. This will help us upstream and cleanup the
>> non-standard properties over the time.
>>
>> Signed-off-by: Srinivas Kandagatla
>> ---
>> drivers/gpu/drm/msm/hdmi/hdmi.c | 35 +--
>> 1 file changed, 25 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c
>> b/drivers/gpu/drm/msm/hdmi/hdmi.c
>> index 8145362..e918889 100644
>> --- a/drivers/gpu/drm/msm/hdmi/hdmi.c
>> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
>> @@ -339,19 +339,34 @@ static const struct of_device_id dt_match[] = {
>> };
>>
>> #ifdef CONFIG_OF
>> +/* This code will be removed once we move to gpiod based calls */
>
> Why don't you do this now instead of duplicating what is essentially
> already implemented in gpiolib?
>
One of the thing that Rob asked in his comments
(http://www.spinics.net/lists/arm-kernel/msg437675.html) was to retain
the support for old devices, moving to gpiod ATM would break such
devices as they are still using legacy gpiolib and its naming.
If Rob is ok to drop gpio properties which does not have "-gpio" or
"-gpios" suffix then we can get rid of this function all together.
--srini
> Thierry
>
[PATCH RFC 5/5] drm/msm/hdmi: remove _clk suffix from clock names.
This patch modifies the driver to support clock names without _clk
suffix, usage of clk names with _clk suffix seems to be non-standard and
picked up everytime in DT patch review.
So lets fix this and make the other clk names deprecated till we decide
to remove them forever.
Signed-off-by: Srinivas Kandagatla
---
drivers/gpu/drm/msm/hdmi/hdmi.c | 29 -
1 file changed, 24 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index e918889..c454d32 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -69,6 +69,24 @@ static void hdmi_destroy(struct hdmi *hdmi)
platform_set_drvdata(hdmi->pdev, NULL);
}
+static struct clk *hdmi_clk_get(struct device *dev, const char *id)
+{
+ char clk_name[32];
+ struct clk *clk;
+
+ snprintf(clk_name, sizeof(clk_name), "%s_clk", id);
+ clk = devm_clk_get(dev, clk_name);
+ if (IS_ERR(clk)) {
+ clk = devm_clk_get(dev, id);
+ if (IS_ERR(clk))
+ return ERR_CAST(clk);
+ } else {
+ dev_warn(dev, "binding deprecated for %s\n", clk_name);
+ }
+
+ return clk;
+}
+
/* construct hdmi at bind/probe time, grab all the resources. If
* we are to EPROBE_DEFER we want to do it here, rather than later
* at modeset_init() time
@@ -158,7 +176,7 @@ static struct hdmi *hdmi_init(struct platform_device *pdev)
for (i = 0; i < config->hpd_clk_cnt; i++) {
struct clk *clk;
- clk = devm_clk_get(&pdev->dev, config->hpd_clk_names[i]);
+ clk = hdmi_clk_get(&pdev->dev, config->hpd_clk_names[i]);
if (IS_ERR(clk)) {
ret = PTR_ERR(clk);
dev_err(&pdev->dev, "failed to get hpd clk: %s (%d)\n",
@@ -175,10 +193,11 @@ static struct hdmi *hdmi_init(struct platform_device
*pdev)
ret = -ENOMEM;
goto fail;
}
+
for (i = 0; i < config->pwr_clk_cnt; i++) {
struct clk *clk;
- clk = devm_clk_get(&pdev->dev, config->pwr_clk_names[i]);
+ clk = hdmi_clk_get(&pdev->dev, config->pwr_clk_names[i]);
if (IS_ERR(clk)) {
ret = PTR_ERR(clk);
dev_err(&pdev->dev, "failed to get pwr clk: %s (%d)\n",
@@ -296,7 +315,7 @@ static struct hdmi_platform_config hdmi_tx_8660_config = {
};
static const char *hpd_reg_names_8960[] = {"core-vdda", "hdmi-mux"};
-static const char *hpd_clk_names_8960[] = {"core_clk", "master_iface_clk",
"slave_iface_clk"};
+static const char *hpd_clk_names_8960[] = {"core", "master_iface",
"slave_iface"};
static struct hdmi_platform_config hdmi_tx_8960_config = {
.phy_init = hdmi_phy_8960_init,
@@ -306,8 +325,8 @@ static struct hdmi_platform_config hdmi_tx_8960_config = {
static const char *pwr_reg_names_8x74[] = {"core-vdda", "core-vcc"};
static const char *hpd_reg_names_8x74[] = {"hpd-gdsc", "hpd-5v"};
-static const char *pwr_clk_names_8x74[] = {"extp_clk", "alt_iface_clk"};
-static const char *hpd_clk_names_8x74[] = {"iface_clk", "core_clk",
"mdp_core_clk"};
+static const char *pwr_clk_names_8x74[] = {"extp", "alt_iface"};
+static const char *hpd_clk_names_8x74[] = {"iface", "core", "mdp_core"};
static unsigned long hpd_clk_freq_8x74[] = {0, 1920, 0};
static struct hdmi_platform_config hdmi_tx_8074_config = {
--
1.9.1
[PATCH RFC 4/5] drm/msm/hdmi: deprecate non standard clock-names
This patch updates the bindings to discourage the usage of non standard clock names, this will help in projects focused on upstreaming. These deprecated properties are still supported but will be remove over the time. Signed-off-by: Srinivas Kandagatla --- Documentation/devicetree/bindings/drm/msm/hdmi.txt | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Documentation/devicetree/bindings/drm/msm/hdmi.txt b/Documentation/devicetree/bindings/drm/msm/hdmi.txt index 6dc202e..6fbfdd8 100644 --- a/Documentation/devicetree/bindings/drm/msm/hdmi.txt +++ b/Documentation/devicetree/bindings/drm/msm/hdmi.txt @@ -12,16 +12,16 @@ Required properties: - clocks: device clocks - clock-names: Corresponding name for each entry in the clocks property. for "qcom,hdmi-tx-8960" compatible names should be - "core_clk" - "master_iface_clk" - "slave_iface_clk" + "core_clk" is deprecated, use "core" instead + "master_iface_clk" is deprecated, use "master_iface" instead + "slave_iface_clk" is deprecated, use "slave_iface" instead for "qcom,hdmi-tx-8084" and "qcom,hdmi-tx-8074" compatible names should be - "extp_clk" - "alt_iface_clk" - "iface_clk" - "core_clk" - "mdp_core_clk" + "extp_clk" is deprecated, use "extp" instead + "alt_iface_clk" is deprecated, use "alt_iface" intstead + "iface_clk" is deprecated, use "iface" instead + "core_clk" is deprecated, use "core" instead + "mdp_core_clk" is deprecated, use "mdp_core" instead See ../clocks/clock-bindings.txt for details. - qcom,hdmi-tx-ddc-clk-gpios: ddc clk pin -- 1.9.1
[PATCH RFC 3/5] drm/msm/hdmi: update bindings to include clock-names.
This patch updates the bindings to include the clock names used in the driver, this would make it easy to add deprecated warning once we move to use standard clock properties. Signed-off-by: Srinivas Kandagatla --- Documentation/devicetree/bindings/drm/msm/hdmi.txt | 13 + 1 file changed, 13 insertions(+) diff --git a/Documentation/devicetree/bindings/drm/msm/hdmi.txt b/Documentation/devicetree/bindings/drm/msm/hdmi.txt index acba581..6dc202e 100644 --- a/Documentation/devicetree/bindings/drm/msm/hdmi.txt +++ b/Documentation/devicetree/bindings/drm/msm/hdmi.txt @@ -10,6 +10,19 @@ Required properties: - reg-names: "core_physical" - interrupts: The interrupt signal from the hdmi block. - clocks: device clocks +- clock-names: Corresponding name for each entry in the clocks property. + for "qcom,hdmi-tx-8960" compatible names should be + "core_clk" + "master_iface_clk" + "slave_iface_clk" + + for "qcom,hdmi-tx-8084" and "qcom,hdmi-tx-8074" compatible names should be + "extp_clk" + "alt_iface_clk" + "iface_clk" + "core_clk" + "mdp_core_clk" + See ../clocks/clock-bindings.txt for details. - qcom,hdmi-tx-ddc-clk-gpios: ddc clk pin - qcom,hdmi-tx-ddc-data-gpios: ddc data pin -- 1.9.1
[PATCH RFC 2/5] drm/msm/hdmi: make use of standard gpio properties.
This patch modifies the driver to support standard gpio properties along
with deprecated properties. This will help us upstream and cleanup the
non-standard properties over the time.
Signed-off-by: Srinivas Kandagatla
---
drivers/gpu/drm/msm/hdmi/hdmi.c | 35 +--
1 file changed, 25 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 8145362..e918889 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -339,19 +339,34 @@ static const struct of_device_id dt_match[] = {
};
#ifdef CONFIG_OF
+/* This code will be removed once we move to gpiod based calls */
static int get_gpio(struct device *dev, struct device_node *of_node, const
char *name)
{
+ char name2[32];
int gpio = of_get_named_gpio(of_node, name, 0);
- if (gpio < 0) {
- char name2[32];
- snprintf(name2, sizeof(name2), "%s-gpio", name);
- gpio = of_get_named_gpio(of_node, name2, 0);
- if (gpio < 0) {
- dev_err(dev, "failed to get gpio: %s (%d)\n",
- name, gpio);
- gpio = -1;
- }
- }
+
+ if (gpio_is_valid(gpio))
+ goto deprecated;
+
+ snprintf(name2, sizeof(name2), "%s-gpio", name);
+ gpio = of_get_named_gpio(of_node, name2, 0);
+
+ if (gpio_is_valid(gpio))
+ goto deprecated;
+
+
+ snprintf(name2, sizeof(name2), "%s-gpios", name);
+ gpio = of_get_named_gpio(of_node, name2, 0);
+
+ if (gpio_is_valid(gpio))
+ return gpio;
+
+ dev_err(dev, "failed to get gpio: %s (%d)\n", name, gpio);
+
+ return -1;
+
+deprecated:
+ dev_warn(dev, "binding deprecated for %s\n", name);
return gpio;
}
#endif
--
1.9.1
[PATCH RFC 1/5] drm/msm/hdmi: deprecate non standard gpio properties.
This patch updates the bindings to discourage the usage of non standard gpio properites, this will help in projects focused on upstreaming. These deprecated properties are still supported but will be remove over the time. Signed-off-by: Srinivas Kandagatla --- Documentation/devicetree/bindings/drm/msm/hdmi.txt | 22 +- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/Documentation/devicetree/bindings/drm/msm/hdmi.txt b/Documentation/devicetree/bindings/drm/msm/hdmi.txt index c43aa53..acba581 100644 --- a/Documentation/devicetree/bindings/drm/msm/hdmi.txt +++ b/Documentation/devicetree/bindings/drm/msm/hdmi.txt @@ -11,15 +11,27 @@ Required properties: - interrupts: The interrupt signal from the hdmi block. - clocks: device clocks See ../clocks/clock-bindings.txt for details. -- qcom,hdmi-tx-ddc-clk-gpio: ddc clk pin -- qcom,hdmi-tx-ddc-data-gpio: ddc data pin -- qcom,hdmi-tx-hpd-gpio: hpd pin +- qcom,hdmi-tx-ddc-clk-gpios: ddc clk pin +- qcom,hdmi-tx-ddc-data-gpios: ddc data pin +- qcom,hdmi-tx-hpd-gpios: hpd pin - core-vdda-supply: phandle to supply regulator - hdmi-mux-supply: phandle to mux regulator +- qcom,hdmi-tx-ddc-clk-gpio: (deprecated) use + "qcom,hdmi-tx-ddc-clk-gpios" instead +- qcom,hdmi-tx-ddc-data-gpio: (deprecated) use + "qcom,hdmi-tx-ddc-data-gpios" instead +- qcom,hdmi-tx-hpd-gpio: (deprecated) use + "qcom,hdmi-tx-hpd-gpios" instead + Optional properties: -- qcom,hdmi-tx-mux-en-gpio: hdmi mux enable pin -- qcom,hdmi-tx-mux-sel-gpio: hdmi mux select pin +- qcom,hdmi-tx-mux-en-gpios: hdmi mux enable pin +- qcom,hdmi-tx-mux-sel-gpios: hdmi mux select pin + +- qcom,hdmi-tx-mux-en-gpio: (deprecated) use "qcom,hdmi-tx-mux-en-gpios" + instead +- qcom,hdmi-tx-mux-sel-gpio: (deprecated) use "qcom,hdmi-tx-mux-sel-gpio" + instead - pinctrl-names: the pin control state names; should contain "default" - pinctrl-0: the default pinctrl state (active) - pinctrl-1: the "sleep" pinctrl state -- 1.9.1
[PATCH RFC 0/5] drm/msm/hdmi: DT bindings cleanup
Hi Rob, Almost every time when we submit the DT changes for hdmi, we are asked why are we using non-standard dt properties. This patchset marks the old style dt properties as deprecated, and let the driver support latest *-gpios and clk names properties. This should allow us to submit the DT patches with more standard dt properites Old style properties are still supported and will be removed once we see no users for it. I have tested these changes on IFC6410. Thanks, srini Srinivas Kandagatla (5): drm/msm/hdmi: deprecate non standard gpio properties. drm/msm/hdmi: make use of standard gpio properties. drm/msm/hdmi: update bindings to include clock-names. drm/msm/hdmi: deprecate non standard clock-names drm/msm/hdmi: remove _clk suffix from clock names. Documentation/devicetree/bindings/drm/msm/hdmi.txt | 35 ++-- drivers/gpu/drm/msm/hdmi/hdmi.c| 64 +- 2 files changed, 79 insertions(+), 20 deletions(-) -- 1.9.1
[PATCH] drm/msm: add calls to prepare and unprepare panel
Prepare the panel before it's enabled and un-prepare after disable, this
will make sure that the regulators are switched on and off correctly.
Tested it on APQ8064 based IFC6410 with panel.
Signed-off-by: Srinivas Kandagatla
---
drivers/gpu/drm/msm/mdp/mdp4/mdp4_lcdc_encoder.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_lcdc_encoder.c
b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_lcdc_encoder.c
index c048433..4cd6e72 100644
--- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_lcdc_encoder.c
+++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_lcdc_encoder.c
@@ -346,8 +346,10 @@ static void mdp4_lcdc_encoder_disable(struct drm_encoder
*encoder)
mdp4_write(mdp4_kms, REG_MDP4_LCDC_ENABLE, 0);
- if (panel)
+ if (panel) {
drm_panel_disable(panel);
+ drm_panel_unprepare(panel);
+ }
/*
* Wait for a vsync so we know the ENABLE=0 latched before
@@ -412,8 +414,10 @@ static void mdp4_lcdc_encoder_enable(struct drm_encoder
*encoder)
if (ret)
dev_err(dev->dev, "failed to enable lcdc_clk: %d\n", ret);
- if (panel)
+ if (panel) {
+ drm_panel_prepare(panel);
drm_panel_enable(panel);
+ }
setup_phy(encoder);
--
1.9.1
[PATCH 1/3] drm/msm: dsi host: add missing of_node_put()
On 22/06/15 15:54, Archit Taneja wrote:
> Decrement device node refcount if of_get_child_by_name is successfully
> called.
>
> Signed-off-by: Archit Taneja
> ---
> drivers/gpu/drm/msm/dsi/dsi_host.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index de04009..1751659 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -1582,6 +1582,8 @@ int msm_dsi_host_register(struct mipi_dsi_host *host,
> bool check_defer)
> node = of_get_child_by_name(msm_host->pdev->dev.of_node,
> "panel");
> if (node) {
> + of_node_put(node);
> +
Atleast from the current state of code, It looks like the driver is
going to refer to the node of_node_put?, So I think this is not the
right place to have of_node_put.
> if (!of_drm_find_panel(node))
> return -EPROBE_DEFER;
> }
>
[PATCH V2] drm/exynos: Add match table for drm platform device
On 21/09/12 19:37, Leela Krishna Amudala wrote:
> This patch is a part of moving the driver to support DT style probing
> of exynos drm device. The compatible name should match with the
> entry in the dtsi file.
>
> Signed-off-by: Leela Krishna Amudala
> ---
> drivers/gpu/drm/exynos/exynos_drm_drv.c | 11 +++
> 1 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index d070719..495be89 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -294,12 +294,23 @@ static int exynos_drm_platform_remove(struct
> platform_device *pdev)
> return 0;
> }
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id drm_device_dt_match[] = {
> + { .compatible = "samsung,exynos-drm-device"},
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, drm_device_dt_match);
> +#else
> +#define drm_device_dt_match NULL
> +#endif
No need of else here as you are using of_match_ptr.
> +
> static struct platform_driver exynos_drm_platform_driver = {
> .probe = exynos_drm_platform_probe,
> .remove = __devexit_p(exynos_drm_platform_remove),
> .driver = {
> .owner = THIS_MODULE,
> .name = "exynos-drm",
> + .of_match_table = of_match_ptr(drm_device_dt_match),
> },
> };
>
Re: [PATCH V2] drm/exynos: Add match table for drm platform device
On 21/09/12 19:37, Leela Krishna Amudala wrote:
> This patch is a part of moving the driver to support DT style probing
> of exynos drm device. The compatible name should match with the
> entry in the dtsi file.
>
> Signed-off-by: Leela Krishna Amudala
> ---
> drivers/gpu/drm/exynos/exynos_drm_drv.c | 11 +++
> 1 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index d070719..495be89 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -294,12 +294,23 @@ static int exynos_drm_platform_remove(struct
> platform_device *pdev)
> return 0;
> }
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id drm_device_dt_match[] = {
> + { .compatible = "samsung,exynos-drm-device"},
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, drm_device_dt_match);
> +#else
> +#define drm_device_dt_match NULL
> +#endif
No need of else here as you are using of_match_ptr.
> +
> static struct platform_driver exynos_drm_platform_driver = {
> .probe = exynos_drm_platform_probe,
> .remove = __devexit_p(exynos_drm_platform_remove),
> .driver = {
> .owner = THIS_MODULE,
> .name = "exynos-drm",
> + .of_match_table = of_match_ptr(drm_device_dt_match),
> },
> };
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
