* Re: [PATCH v14 2/4] CMDQ: Mediatek CMDQ driver
From: Horng-Shyang Liao @ 2016-09-30 9:47 UTC (permalink / raw)
To: CK Hu
Cc: Rob Herring, Matthias Brugger, Jassi Brar, Daniel Kurtz,
Sascha Hauer, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
srv_heupstream-NuS5LvNUpcJWk0Htik3J/w, Sascha Hauer,
Philipp Zabel, Nicolas Boichat, cawa cheng, Bibby Hsieh, YT Shen,
Daoyuan Huang, Damon Chu, Josh-YC Liu, Glory Hung
In-Reply-To: <1475226691.13398.35.camel@mtksdaap41>
On Fri, 2016-09-30 at 17:11 +0800, CK Hu wrote:
> Hi, HS:
>
> One comment inline
>
> On Fri, 2016-09-30 at 16:56 +0800, Horng-Shyang Liao wrote:
> > Hi CK,
> >
> > Please see my inline reply.
> >
> > On Fri, 2016-09-30 at 11:06 +0800, CK Hu wrote:
> > > Hi, HS:
> > >
> > > On Mon, 2016-09-05 at 09:44 +0800, HS Liao wrote:
> > > > This patch is first version of Mediatek Command Queue(CMDQ) driver. The
> > > > CMDQ is used to help write registers with critical time limitation,
> > > > such as updating display configuration during the vblank. It controls
> > > > Global Command Engine (GCE) hardware to achieve this requirement.
> > > > Currently, CMDQ only supports display related hardwares, but we expect
> > > > it can be extended to other hardwares for future requirements.
> > > >
> > > > Signed-off-by: HS Liao <hs.liao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > > > Signed-off-by: CK Hu <ck.hu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > > > ---
> > >
> > > [snip...]
> > >
> > > > +
> > > > +struct cmdq_task {
> > > > + struct cmdq *cmdq;
> > > > + struct list_head list_entry;
> > > > + void *va_base;
> > > > + dma_addr_t pa_base;
> > > > + size_t cmd_buf_size; /* command occupied size */
> > > > + size_t buf_size; /* real buffer size */
> > > > + bool finalized;
> > > > + struct cmdq_thread *thread;
> > >
> > > I think thread info could be removed from cmdq_task. Only
> > > cmdq_task_handle_error() and cmdq_task_insert_into_thread() use
> > > task->thread and caller of both function has the thread info. So you
> > > could just pass thread info into these two function and remove thread
> > > info in cmdq_task.
> >
> > This modification will remove 1 pointer but add 2 pointers. Moreover,
> > more pointers will need to be delivered between functions for future
> > extension. IMHO, it would be better to keep thread pointer inside
> > cmdq_task.
> >
> > > > + struct cmdq_task_cb cb;
> > >
> > > I think this callback function is equal to mailbox client tx_done
> > > callback. It's better to use already-defined interface rather than
> > > creating your own.
> >
> > This is because CMDQ driver allows different callback functions for
> > different tasks, but mailbox only allows one callback function per
> > channel. But, I think I can add a wrapper for tx_done to call CMDQ
> > callback functions. So, I will use tx_done in CMDQ v15.
>
> Up to now, one callback function for one channel is enough for DRM. So
> 'different callback function for different sent-message' looks like an
> advanced function. Maybe you should not include it in first patch.
>
> Regards,
> CK
Hi CK,
OK. I will do it.
Thanks,
HS
[snip...]
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v14 2/4] CMDQ: Mediatek CMDQ driver
From: CK Hu @ 2016-09-30 9:11 UTC (permalink / raw)
To: Horng-Shyang Liao
Cc: Rob Herring, Matthias Brugger, Jassi Brar, Daniel Kurtz,
Sascha Hauer, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
srv_heupstream-NuS5LvNUpcJWk0Htik3J/w, Sascha Hauer,
Philipp Zabel, Nicolas Boichat, cawa cheng, Bibby Hsieh, YT Shen,
Daoyuan Huang, Damon Chu, Josh-YC Liu, Glory Hung
In-Reply-To: <1475225778.25044.35.camel@mtksdaap41>
Hi, HS:
One comment inline
On Fri, 2016-09-30 at 16:56 +0800, Horng-Shyang Liao wrote:
> Hi CK,
>
> Please see my inline reply.
>
> On Fri, 2016-09-30 at 11:06 +0800, CK Hu wrote:
> > Hi, HS:
> >
> > On Mon, 2016-09-05 at 09:44 +0800, HS Liao wrote:
> > > This patch is first version of Mediatek Command Queue(CMDQ) driver. The
> > > CMDQ is used to help write registers with critical time limitation,
> > > such as updating display configuration during the vblank. It controls
> > > Global Command Engine (GCE) hardware to achieve this requirement.
> > > Currently, CMDQ only supports display related hardwares, but we expect
> > > it can be extended to other hardwares for future requirements.
> > >
> > > Signed-off-by: HS Liao <hs.liao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > > Signed-off-by: CK Hu <ck.hu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > > ---
> >
> > [snip...]
> >
> > > +
> > > +struct cmdq_task {
> > > + struct cmdq *cmdq;
> > > + struct list_head list_entry;
> > > + void *va_base;
> > > + dma_addr_t pa_base;
> > > + size_t cmd_buf_size; /* command occupied size */
> > > + size_t buf_size; /* real buffer size */
> > > + bool finalized;
> > > + struct cmdq_thread *thread;
> >
> > I think thread info could be removed from cmdq_task. Only
> > cmdq_task_handle_error() and cmdq_task_insert_into_thread() use
> > task->thread and caller of both function has the thread info. So you
> > could just pass thread info into these two function and remove thread
> > info in cmdq_task.
>
> This modification will remove 1 pointer but add 2 pointers. Moreover,
> more pointers will need to be delivered between functions for future
> extension. IMHO, it would be better to keep thread pointer inside
> cmdq_task.
>
> > > + struct cmdq_task_cb cb;
> >
> > I think this callback function is equal to mailbox client tx_done
> > callback. It's better to use already-defined interface rather than
> > creating your own.
>
> This is because CMDQ driver allows different callback functions for
> different tasks, but mailbox only allows one callback function per
> channel. But, I think I can add a wrapper for tx_done to call CMDQ
> callback functions. So, I will use tx_done in CMDQ v15.
Up to now, one callback function for one channel is enough for DRM. So
'different callback function for different sent-message' looks like an
advanced function. Maybe you should not include it in first patch.
Regards,
CK
>
> > > +};
> > > +
> >
> > [snip...]
> >
> > > +
> > > +static int cmdq_suspend(struct device *dev)
> > > +{
> > > + struct cmdq *cmdq = dev_get_drvdata(dev);
> > > + struct cmdq_thread *thread;
> > > + int i;
> > > + bool task_running = false;
> > > +
> > > + mutex_lock(&cmdq->task_mutex);
> > > + cmdq->suspended = true;
> > > + mutex_unlock(&cmdq->task_mutex);
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(cmdq->thread); i++) {
> > > + thread = &cmdq->thread[i];
> > > + if (!list_empty(&thread->task_busy_list)) {
> > > + mod_timer(&thread->timeout, jiffies + 1);
> > > + task_running = true;
> > > + }
> > > + }
> > > +
> > > + if (task_running) {
> > > + dev_warn(dev, "exist running task(s) in suspend\n");
> > > + msleep(20);
> >
> > Why sleep here? It looks like a recovery but could 20ms recovery
> > something? I think warning message is enough because you see the warning
> > message, and you fix the bug, so no need to recovery anything.
>
> My purpose is context switch to finish timer's work.
> I will replace it by schedule().
>
> > > + }
> > > +
> > > + clk_unprepare(cmdq->clock);
> > > + return 0;
> > > +}
> > > +
> >
> > Regards,
> > CK
>
> Thanks,
> HS
>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v14 4/4] CMDQ: save more energy in idle
From: Matthias Brugger @ 2016-09-30 9:01 UTC (permalink / raw)
To: Horng-Shyang Liao, Jassi Brar
Cc: Rob Herring, Daniel Kurtz, Sascha Hauer, Devicetree List,
Linux Kernel Mailing List,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
srv_heupstream-NuS5LvNUpcJWk0Htik3J/w, Sascha Hauer,
Philipp Zabel, Nicolas Boichat, CK HU, cawa cheng, Bibby Hsieh,
YT Shen, Daoyuan Huang, Damon Chu, Josh-YC Liu,
Glory Hung <glor>
In-Reply-To: <1475225766.25044.33.camel@mtksdaap41>
On 09/30/2016 10:56 AM, Horng-Shyang Liao wrote:
> On Fri, 2016-09-23 at 17:28 +0800, Horng-Shyang Liao wrote:
>> On Thu, 2016-09-22 at 13:22 +0530, Jassi Brar wrote:
>>> On Mon, Sep 5, 2016 at 7:14 AM, HS Liao <hs.liao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
>>>> Use clk_disable_unprepare instead of clk_disable to save more energy
>>>> when CMDQ is idle.
>>>>
>>>> Signed-off-by: HS Liao <hs.liao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>>>> ---
>>>> drivers/mailbox/mtk-cmdq.c | 54 +++++++++++++++++++++++++++++++++++++++-------
>>>
>>> The driver is introduced by second patch of the set, so it makes sense
>>> to merge this patch into patch 2/4.
>>
>> Hi Jassi,
>>
>> Could you take a look at previous discussion between Matthias and me?
>> http://lkml.iu.edu/hypermail/linux/kernel/1606.2/05239.html
>> His basic idea is to simplify first working version.
>> Therefore, I move some code to this patch.
>>
Well what I wanted to say is, that right now this driver is quite a big
beast and this makes it difficult to review. So my idea was to just
submit the most basic version of this driver.
Any improvements on the driver should be sent in follow-up patches after
the basic driver got merged. That was my idea.
Regards,
Matthias
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v14 2/4] CMDQ: Mediatek CMDQ driver
From: Horng-Shyang Liao @ 2016-09-30 8:56 UTC (permalink / raw)
To: CK Hu
Cc: Monica Wang, Jiaguang Zhang, Nicolas Boichat, Jassi Brar,
cawa cheng, hs.liao-NuS5LvNUpcJWk0Htik3J/w, Bibby Hsieh, YT Shen,
Damon Chu, devicetree-u79uwXL29TY76Z2rM5mHXA, Sascha Hauer,
Daoyuan Huang, Sascha Hauer, Glory Hung, Rob Herring,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Matthias Brugger,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
srv_heupstream-NuS5LvNUpcJWk0Htik3J/w, Josh-YC Liu,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Dennis-YC Hsieh,
Philipp Zabel
In-Reply-To: <1475204778.13398.28.camel@mtksdaap41>
Hi CK,
Please see my inline reply.
On Fri, 2016-09-30 at 11:06 +0800, CK Hu wrote:
> Hi, HS:
>
> On Mon, 2016-09-05 at 09:44 +0800, HS Liao wrote:
> > This patch is first version of Mediatek Command Queue(CMDQ) driver. The
> > CMDQ is used to help write registers with critical time limitation,
> > such as updating display configuration during the vblank. It controls
> > Global Command Engine (GCE) hardware to achieve this requirement.
> > Currently, CMDQ only supports display related hardwares, but we expect
> > it can be extended to other hardwares for future requirements.
> >
> > Signed-off-by: HS Liao <hs.liao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > Signed-off-by: CK Hu <ck.hu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > ---
>
> [snip...]
>
> > +
> > +struct cmdq_task {
> > + struct cmdq *cmdq;
> > + struct list_head list_entry;
> > + void *va_base;
> > + dma_addr_t pa_base;
> > + size_t cmd_buf_size; /* command occupied size */
> > + size_t buf_size; /* real buffer size */
> > + bool finalized;
> > + struct cmdq_thread *thread;
>
> I think thread info could be removed from cmdq_task. Only
> cmdq_task_handle_error() and cmdq_task_insert_into_thread() use
> task->thread and caller of both function has the thread info. So you
> could just pass thread info into these two function and remove thread
> info in cmdq_task.
This modification will remove 1 pointer but add 2 pointers. Moreover,
more pointers will need to be delivered between functions for future
extension. IMHO, it would be better to keep thread pointer inside
cmdq_task.
> > + struct cmdq_task_cb cb;
>
> I think this callback function is equal to mailbox client tx_done
> callback. It's better to use already-defined interface rather than
> creating your own.
This is because CMDQ driver allows different callback functions for
different tasks, but mailbox only allows one callback function per
channel. But, I think I can add a wrapper for tx_done to call CMDQ
callback functions. So, I will use tx_done in CMDQ v15.
> > +};
> > +
>
> [snip...]
>
> > +
> > +static int cmdq_suspend(struct device *dev)
> > +{
> > + struct cmdq *cmdq = dev_get_drvdata(dev);
> > + struct cmdq_thread *thread;
> > + int i;
> > + bool task_running = false;
> > +
> > + mutex_lock(&cmdq->task_mutex);
> > + cmdq->suspended = true;
> > + mutex_unlock(&cmdq->task_mutex);
> > +
> > + for (i = 0; i < ARRAY_SIZE(cmdq->thread); i++) {
> > + thread = &cmdq->thread[i];
> > + if (!list_empty(&thread->task_busy_list)) {
> > + mod_timer(&thread->timeout, jiffies + 1);
> > + task_running = true;
> > + }
> > + }
> > +
> > + if (task_running) {
> > + dev_warn(dev, "exist running task(s) in suspend\n");
> > + msleep(20);
>
> Why sleep here? It looks like a recovery but could 20ms recovery
> something? I think warning message is enough because you see the warning
> message, and you fix the bug, so no need to recovery anything.
My purpose is context switch to finish timer's work.
I will replace it by schedule().
> > + }
> > +
> > + clk_unprepare(cmdq->clock);
> > + return 0;
> > +}
> > +
>
> Regards,
> CK
Thanks,
HS
^ permalink raw reply
* Re: [PATCH v14 2/4] CMDQ: Mediatek CMDQ driver
From: Horng-Shyang Liao @ 2016-09-30 8:56 UTC (permalink / raw)
To: Jassi Brar
Cc: Rob Herring, Matthias Brugger, Daniel Kurtz, Sascha Hauer,
Devicetree List, Linux Kernel Mailing List,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
srv_heupstream-NuS5LvNUpcJWk0Htik3J/w, Sascha Hauer,
Philipp Zabel, Nicolas Boichat, CK HU, cawa cheng, Bibby Hsieh,
YT Shen, Daoyuan Huang, Damon Chu, Jo
In-Reply-To: <1474622898.21723.26.camel@mtksdaap41>
On Fri, 2016-09-23 at 17:28 +0800, Horng-Shyang Liao wrote:
> Hi Jassi,
>
> Please see my inline reply.
>
> On Thu, 2016-09-22 at 13:47 +0530, Jassi Brar wrote:
> > On Mon, Sep 5, 2016 at 7:14 AM, HS Liao <hs.liao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> [...]
> > > +struct cmdq_base *cmdq_register_device(struct device *dev)
> > > +{
> > > + struct cmdq_base *cmdq_base;
> > > + struct resource res;
> > > + int subsys;
> > > + u32 base;
> > > +
> > > + if (of_address_to_resource(dev->of_node, 0, &res))
> > > + return NULL;
> > > + base = (u32)res.start;
> > > +
> > > + subsys = cmdq_subsys_base_to_id(base >> 16);
> > > + if (subsys < 0)
> > > + return NULL;
> > > +
> > > + cmdq_base = devm_kmalloc(dev, sizeof(*cmdq_base), GFP_KERNEL);
> > > + if (!cmdq_base)
> > > + return NULL;
> > > + cmdq_base->subsys = subsys;
> > > + cmdq_base->base = base;
> > > +
> > > + return cmdq_base;
> > > +}
> > > +EXPORT_SYMBOL(cmdq_register_device);
> > > +
> > > +struct cmdq_client *cmdq_mbox_create(struct device *dev, int index)
> > > +{
> > > + struct cmdq_client *client;
> > > +
> > > + client = kzalloc(sizeof(*client), GFP_KERNEL);
> > > + client->client.dev = dev;
> > > + client->client.tx_block = false;
> > > + client->chan = mbox_request_channel(&client->client, index);
> > > + return client;
> > > +}
> > > +EXPORT_SYMBOL(cmdq_mbox_create);
> > > +
> > > +int cmdq_task_create(struct device *dev, struct cmdq_task **task_ptr)
> > > +{
> > > + struct cmdq_task *task;
> > > + int err;
> > > +
> > > + task = kzalloc(sizeof(*task), GFP_KERNEL);
> > > + if (!task)
> > > + return -ENOMEM;
> > > + task->cmdq = dev_get_drvdata(dev);
> > > + err = cmdq_task_realloc_cmd_buffer(task, PAGE_SIZE);
> > > + if (err < 0) {
> > > + kfree(task);
> > > + return err;
> > > + }
> > > + *task_ptr = task;
> > > + return 0;
> > > +}
> > > +EXPORT_SYMBOL(cmdq_task_create);
> > > +
> > > +static int cmdq_task_append_command(struct cmdq_task *task, enum cmdq_code code,
> > > + u32 arg_a, u32 arg_b)
> > > +{
> > > + u64 *cmd_ptr;
> > > + int err;
> > > +
> > > + if (WARN_ON(task->finalized))
> > > + return -EBUSY;
> > > + if (unlikely(task->cmd_buf_size + CMDQ_INST_SIZE > task->buf_size)) {
> > > + err = cmdq_task_realloc_cmd_buffer(task, task->buf_size * 2);
> > > + if (err < 0)
> > > + return err;
> > > + }
> > > + cmd_ptr = task->va_base + task->cmd_buf_size;
> > > + (*cmd_ptr) = (u64)((code << CMDQ_OP_CODE_SHIFT) | arg_a) << 32 | arg_b;
> > > + task->cmd_buf_size += CMDQ_INST_SIZE;
> > > + return 0;
> > > +}
> > > +
> > > +int cmdq_task_write(struct cmdq_task *task, u32 value, struct cmdq_base *base,
> > > + u32 offset)
> > > +{
> > > + u32 arg_a = ((base->base + offset) & CMDQ_ARG_A_WRITE_MASK) |
> > > + (base->subsys << CMDQ_SUBSYS_SHIFT);
> > > + return cmdq_task_append_command(task, CMDQ_CODE_WRITE, arg_a, value);
> > > +}
> > > +EXPORT_SYMBOL(cmdq_task_write);
> > > +
> > > +int cmdq_task_write_mask(struct cmdq_task *task, u32 value,
> > > + struct cmdq_base *base, u32 offset, u32 mask)
> > > +{
> > > + u32 offset_mask = offset;
> > > + int err;
> > > +
> > > + if (mask != 0xffffffff) {
> > > + err = cmdq_task_append_command(task, CMDQ_CODE_MASK, 0, ~mask);
> > > + if (err < 0)
> > > + return err;
> > > + offset_mask |= CMDQ_WRITE_ENABLE_MASK;
> > > + }
> > > + return cmdq_task_write(task, value, base, offset_mask);
> > > +}
> > > +EXPORT_SYMBOL(cmdq_task_write_mask);
> > > +
> > > +static const u32 cmdq_event_value[CMDQ_MAX_EVENT] = {
> > > + /* Display start of frame(SOF) events */
> > > + [CMDQ_EVENT_DISP_OVL0_SOF] = 11,
> > > + [CMDQ_EVENT_DISP_OVL1_SOF] = 12,
> > > + [CMDQ_EVENT_DISP_RDMA0_SOF] = 13,
> > > + [CMDQ_EVENT_DISP_RDMA1_SOF] = 14,
> > > + [CMDQ_EVENT_DISP_RDMA2_SOF] = 15,
> > > + [CMDQ_EVENT_DISP_WDMA0_SOF] = 16,
> > > + [CMDQ_EVENT_DISP_WDMA1_SOF] = 17,
> > > + /* Display end of frame(EOF) events */
> > > + [CMDQ_EVENT_DISP_OVL0_EOF] = 39,
> > > + [CMDQ_EVENT_DISP_OVL1_EOF] = 40,
> > > + [CMDQ_EVENT_DISP_RDMA0_EOF] = 41,
> > > + [CMDQ_EVENT_DISP_RDMA1_EOF] = 42,
> > > + [CMDQ_EVENT_DISP_RDMA2_EOF] = 43,
> > > + [CMDQ_EVENT_DISP_WDMA0_EOF] = 44,
> > > + [CMDQ_EVENT_DISP_WDMA1_EOF] = 45,
> > > + /* Mutex end of frame(EOF) events */
> > > + [CMDQ_EVENT_MUTEX0_STREAM_EOF] = 53,
> > > + [CMDQ_EVENT_MUTEX1_STREAM_EOF] = 54,
> > > + [CMDQ_EVENT_MUTEX2_STREAM_EOF] = 55,
> > > + [CMDQ_EVENT_MUTEX3_STREAM_EOF] = 56,
> > > + [CMDQ_EVENT_MUTEX4_STREAM_EOF] = 57,
> > > + /* Display underrun events */
> > > + [CMDQ_EVENT_DISP_RDMA0_UNDERRUN] = 63,
> > > + [CMDQ_EVENT_DISP_RDMA1_UNDERRUN] = 64,
> > > + [CMDQ_EVENT_DISP_RDMA2_UNDERRUN] = 65,
> > > +};
> > > +
> > > +int cmdq_task_wfe(struct cmdq_task *task, enum cmdq_event event)
> > > +{
> > > + u32 arg_b;
> > > +
> > > + if (event >= CMDQ_MAX_EVENT || event < 0)
> > > + return -EINVAL;
> > > +
> > > + /*
> > > + * WFE arg_b
> > > + * bit 0-11: wait value
> > > + * bit 15: 1 - wait, 0 - no wait
> > > + * bit 16-27: update value
> > > + * bit 31: 1 - update, 0 - no update
> > > + */
> > > + arg_b = CMDQ_WFE_UPDATE | CMDQ_WFE_WAIT | CMDQ_WFE_WAIT_VALUE;
> > > + return cmdq_task_append_command(task, CMDQ_CODE_WFE,
> > > + cmdq_event_value[event], arg_b);
> > > +}
> > > +EXPORT_SYMBOL(cmdq_task_wfe);
> > > +
> > > +int cmdq_task_clear_event(struct cmdq_task *task, enum cmdq_event event)
> > > +{
> > > + if (event >= CMDQ_MAX_EVENT || event < 0)
> > > + return -EINVAL;
> > > +
> > > + return cmdq_task_append_command(task, CMDQ_CODE_WFE,
> > > + cmdq_event_value[event], CMDQ_WFE_UPDATE);
> > > +}
> > > +EXPORT_SYMBOL(cmdq_task_clear_event);
> > > +
> > > +static int cmdq_task_finalize(struct cmdq_task *task)
> > > +{
> > > + int err;
> > > +
> > > + if (task->finalized)
> > > + return 0;
> > > +
> > > + /* insert EOC and generate IRQ for each command iteration */
> > > + err = cmdq_task_append_command(task, CMDQ_CODE_EOC, 0, CMDQ_EOC_IRQ_EN);
> > > + if (err < 0)
> > > + return err;
> > > +
> > > + /* JUMP to end */
> > > + err = cmdq_task_append_command(task, CMDQ_CODE_JUMP, 0, CMDQ_JUMP_PASS);
> > > + if (err < 0)
> > > + return err;
> > > +
> > > + task->finalized = true;
> > > + return 0;
> > > +}
> > > +
> > > +int cmdq_task_flush_async(struct cmdq_client *client, struct cmdq_task *task,
> > > + cmdq_async_flush_cb cb, void *data)
> > > +{
> > > + struct cmdq *cmdq = task->cmdq;
> > > + int err;
> > > +
> > > + mutex_lock(&cmdq->task_mutex);
> > > + if (cmdq->suspended) {
> > > + dev_err(cmdq->mbox.dev, "%s is called after suspended\n",
> > > + __func__);
> > > + mutex_unlock(&cmdq->task_mutex);
> > > + return -EPERM;
> > > + }
> > > +
> > > + err = cmdq_task_finalize(task);
> > > + if (err < 0) {
> > > + mutex_unlock(&cmdq->task_mutex);
> > > + return err;
> > > + }
> > > +
> > > + INIT_LIST_HEAD(&task->list_entry);
> > > + task->cb.cb = cb;
> > > + task->cb.data = data;
> > > + task->pa_base = dma_map_single(cmdq->mbox.dev, task->va_base,
> > > + task->cmd_buf_size, DMA_TO_DEVICE);
> > > +
> > > + mbox_send_message(client->chan, task);
> > > + /* We can send next task immediately, so just call txdone. */
> > > + mbox_client_txdone(client->chan, 0);
> > > + mutex_unlock(&cmdq->task_mutex);
> > > + return 0;
> > > +}
> > > +EXPORT_SYMBOL(cmdq_task_flush_async);
> > > +
> > > +struct cmdq_flush_completion {
> > > + struct completion cmplt;
> > > + bool err;
> > > +};
> > > +
> > > +static void cmdq_task_flush_cb(struct cmdq_cb_data data)
> > > +{
> > > + struct cmdq_flush_completion *cmplt = data.data;
> > > +
> > > + cmplt->err = data.err;
> > > + complete(&cmplt->cmplt);
> > > +}
> > > +
> > > +int cmdq_task_flush(struct cmdq_client *client, struct cmdq_task *task)
> > > +{
> > > + struct cmdq_flush_completion cmplt;
> > > + int err;
> > > +
> > > + init_completion(&cmplt.cmplt);
> > > + err = cmdq_task_flush_async(client, task, cmdq_task_flush_cb, &cmplt);
> > > + if (err < 0)
> > > + return err;
> > > + wait_for_completion(&cmplt.cmplt);
> > > + return cmplt.err ? -EFAULT : 0;
> > > +}
> > > +EXPORT_SYMBOL(cmdq_task_flush);
> > > +
> > > +void cmdq_mbox_free(struct cmdq_client *client)
> > > +{
> > > + mbox_free_channel(client->chan);
> > > + kfree(client);
> > > +}
> > > +EXPORT_SYMBOL(cmdq_mbox_free);
> > > +
> > All these exported functions implement the protocol, so should not be
> > a part of this controller driver. That should go into
> > drivers/soc/mediatek/
> >
> > The controller driver (mtk-cmdq.c) should implement mainly the
> > mbox_chan_ops and mbox.of_xlate.
> >
>
> I can do that, but I would like to confirm with Matthias in advance.
>
> [...]
> > > + cmdq->irq = irq_of_parse_and_map(node, 0);
> > >
> > why not, cmdq->irq = platform_get_irq(pdev, 0);
>
> Will do
>
> [...]
> > > +static struct platform_driver cmdq_drv = {
> > > + .probe = cmdq_probe,
> > > + .remove = cmdq_remove,
> > > + .driver = {
> > > + .name = "mtk_cmdq",
> > > + .owner = THIS_MODULE,
> > >
> > please remove the unnecessary .owner field.
>
> Will do
>
> > > + .pm = &cmdq_pm_ops,
> > > + .of_match_table = cmdq_of_ids,
> > > + }
> > > +};
> > > +
> > > +builtin_platform_driver(cmdq_drv);
> > > diff --git a/include/linux/mailbox/mtk-cmdq.h b/include/linux/mailbox/mtk-cmdq.h
> > > new file mode 100644
> > > index 0000000..c3c924d
> > > --- /dev/null
> > > +++ b/include/linux/mailbox/mtk-cmdq.h
> > >
> > The api implemented is Mediateck proprietary, so I think it should be
> > include/linux/soc/mediatek/cmdq.h
> >
> >
> > > @@ -0,0 +1,180 @@
> > > +/*
> > > + * Copyright (c) 2015 MediaTek Inc.
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License version 2 as
> > > + * published by the Free Software Foundation.
> > > + *
> > > + * This program is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > > + * GNU General Public License for more details.
> > > + */
> > > +
> > > +#ifndef __MTK_CMDQ_H__
> > > +#define __MTK_CMDQ_H__
> > > +
> > > +#include <linux/mailbox_client.h>
> > > +#include <linux/mailbox_controller.h>
> > >
> > Clients should not need to include mailbox_controller.h
>
> This is because client needs to know controller's dev.
>
> Please see my CMDQ v13.
> http://www.spinics.net/lists/kernel/msg2327867.html
> I add mailbox_controller.h for client to get controller's dev,
> so I can remove a node reference in device tree.
>
> Should I revert the modification of CMDQ v13?
Hi Jassi,
CMDQ clients don't need to know controller device before flush,
and CMDQ driver can get controller device by itself in flushing flow.
So, I think mailbox_controller.h can be removed from here,
and CMDQ v13 doesn't need to be reverted, either.
I will update this part in CMDQ v15.
Thanks,
HS
> > > +#include <linux/platform_device.h>
> > > +#include <linux/types.h>
> > > +
> > > +/* display events in command queue(CMDQ) */
> > > +enum cmdq_event {
> > > + /* Display start of frame(SOF) events */
> > > + CMDQ_EVENT_DISP_OVL0_SOF,
> > >
> > you may want to explicitly initialise the first element.
>
> Will do
>
> > > + CMDQ_EVENT_DISP_OVL1_SOF,
> > > + CMDQ_EVENT_DISP_RDMA0_SOF,
> > > + CMDQ_EVENT_DISP_RDMA1_SOF,
> > > + CMDQ_EVENT_DISP_RDMA2_SOF,
> > > + CMDQ_EVENT_DISP_WDMA0_SOF,
> > > + CMDQ_EVENT_DISP_WDMA1_SOF,
> > > + /* Display end of frame(EOF) events */
> > > + CMDQ_EVENT_DISP_OVL0_EOF,
> > > + CMDQ_EVENT_DISP_OVL1_EOF,
> > > + CMDQ_EVENT_DISP_RDMA0_EOF,
> > > + CMDQ_EVENT_DISP_RDMA1_EOF,
> > > + CMDQ_EVENT_DISP_RDMA2_EOF,
> > > + CMDQ_EVENT_DISP_WDMA0_EOF,
> > > + CMDQ_EVENT_DISP_WDMA1_EOF,
> > > + /* Mutex end of frame(EOF) events */
> > > + CMDQ_EVENT_MUTEX0_STREAM_EOF,
> > > + CMDQ_EVENT_MUTEX1_STREAM_EOF,
> > > + CMDQ_EVENT_MUTEX2_STREAM_EOF,
> > > + CMDQ_EVENT_MUTEX3_STREAM_EOF,
> > > + CMDQ_EVENT_MUTEX4_STREAM_EOF,
> > > + /* Display underrun events */
> > > + CMDQ_EVENT_DISP_RDMA0_UNDERRUN,
> > > + CMDQ_EVENT_DISP_RDMA1_UNDERRUN,
> > > + CMDQ_EVENT_DISP_RDMA2_UNDERRUN,
> > > + /* Keep this at the end */
> > > + CMDQ_MAX_EVENT,
> > > +};
> > > +
>
> Thanks,
> HS
>
>
> Hi Matthias,
>
> Do you agree with Jassi's comments about moving parts of code back to
> soc/mediatek/ ?
> If I do it, the part in soc/mediatek/ will be similar to a library.
> Could you tell me a good way to handle this situation?
>
> Thanks,
> HS
Hi Matthias,
Do you have any suggestion about moving parts of code back to
soc/mediatek/ ?
Thanks,
HS
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v14 4/4] CMDQ: save more energy in idle
From: Horng-Shyang Liao @ 2016-09-30 8:56 UTC (permalink / raw)
To: Jassi Brar
Cc: Rob Herring, Matthias Brugger, Daniel Kurtz, Sascha Hauer,
Devicetree List, Linux Kernel Mailing List,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
srv_heupstream-NuS5LvNUpcJWk0Htik3J/w, Sascha Hauer,
Philipp Zabel, Nicolas Boichat, CK HU, cawa cheng, Bibby Hsieh,
YT Shen, Daoyuan Huang, Damon Chu, Jo
In-Reply-To: <1474622885.21723.25.camel@mtksdaap41>
On Fri, 2016-09-23 at 17:28 +0800, Horng-Shyang Liao wrote:
> On Thu, 2016-09-22 at 13:22 +0530, Jassi Brar wrote:
> > On Mon, Sep 5, 2016 at 7:14 AM, HS Liao <hs.liao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> > > Use clk_disable_unprepare instead of clk_disable to save more energy
> > > when CMDQ is idle.
> > >
> > > Signed-off-by: HS Liao <hs.liao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > > ---
> > > drivers/mailbox/mtk-cmdq.c | 54 +++++++++++++++++++++++++++++++++++++++-------
> >
> > The driver is introduced by second patch of the set, so it makes sense
> > to merge this patch into patch 2/4.
>
> Hi Jassi,
>
> Could you take a look at previous discussion between Matthias and me?
> http://lkml.iu.edu/hypermail/linux/kernel/1606.2/05239.html
> His basic idea is to simplify first working version.
> Therefore, I move some code to this patch.
>
> Thanks,
> HS
>
Hi Jassi,
What do you think about our previous discussion?
Thanks,
HS
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] net: ethernet: mediatek: mark symbols static where possible
From: Baoyou Xie @ 2016-09-30 7:48 UTC (permalink / raw)
To: nbd, blogic, matthias.bgg
Cc: netdev, linux-arm-kernel, linux-mediatek, linux-kernel, arnd,
baoyou.xie, xie.baoyou, han.fei, tang.qiang007
We get 2 warnings when building kernel with W=1:
drivers/net/ethernet/mediatek/mtk_eth_soc.c:2041:5: warning: no previous prototype for 'mtk_get_link_ksettings' [-Wmissing-prototypes]
drivers/net/ethernet/mediatek/mtk_eth_soc.c:2052:5: warning: no previous prototype for 'mtk_set_link_ksettings' [-Wmissing-prototypes]
In fact, these functions are only used in the file in which they are
declared and don't need a declaration, but can be made static.
So this patch marks these functions with 'static'.
Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index ddf20a0..ad4ab97 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -2038,8 +2038,8 @@ static int mtk_cleanup(struct mtk_eth *eth)
return 0;
}
-int mtk_get_link_ksettings(struct net_device *ndev,
- struct ethtool_link_ksettings *cmd)
+static int mtk_get_link_ksettings(struct net_device *ndev,
+ struct ethtool_link_ksettings *cmd)
{
struct mtk_mac *mac = netdev_priv(ndev);
@@ -2049,8 +2049,8 @@ int mtk_get_link_ksettings(struct net_device *ndev,
return phy_ethtool_ksettings_get(ndev->phydev, cmd);
}
-int mtk_set_link_ksettings(struct net_device *ndev,
- const struct ethtool_link_ksettings *cmd)
+static int mtk_set_link_ksettings(struct net_device *ndev,
+ const struct ethtool_link_ksettings *cmd)
{
struct mtk_mac *mac = netdev_priv(ndev);
--
2.7.4
^ permalink raw reply related
* [PATCH] drm/mediatek: stop using drm_vblank_count() as the hw frame counter
From: YT Shen @ 2016-09-30 6:45 UTC (permalink / raw)
To: Philipp Zabel, CK Hu
Cc: Jitao Shi, srv_heupstream, Jie Qiu, Daniel Vetter, linux-kernel,
dri-devel, linux-mediatek, Matthias Brugger, linux-arm-kernel
using drm_vblank_no_hw_counter() to eliminate kernel warning.
Signed-off-by: YT Shen <yt.shen@mediatek.com>
---
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index eebb7d8..e286193 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -239,7 +239,7 @@ static struct drm_driver mtk_drm_driver = {
.driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME |
DRIVER_ATOMIC,
- .get_vblank_counter = drm_vblank_count,
+ .get_vblank_counter = drm_vblank_no_hw_counter,
.enable_vblank = mtk_drm_crtc_enable_vblank,
.disable_vblank = mtk_drm_crtc_disable_vblank,
--
1.9.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related
* Re: [PATCH] drm/mediatek: fix a typo
From: Bibby Hsieh @ 2016-09-30 3:11 UTC (permalink / raw)
To: Matthias Brugger
Cc: CK Hu, David Airlie, Daniel Vetter, dri-devel, linux-mediatek,
Yingjoe Chen, Cawa Cheng, Daniel Kurtz, Philipp Zabel, YT Shen,
Thierry Reding, Mao Huang, linux-arm-kernel, linux-kernel,
Sascha Hauer
In-Reply-To: <1f324471-bebb-c8bf-75c1-391355c50a99@gmail.com>
On Thu, 2016-09-29 at 10:46 +0200, Matthias Brugger wrote:
>
> On 29/09/16 06:01, CK Hu wrote:
> > Acked-by: CK Hu <ck.hu@mediatek.com>
> >
> > On Thu, 2016-09-29 at 11:22 +0800, Bibby Hsieh wrote:
> >> Fix the typo: OD_RELAYMODE->OD_CFG
> >>
>
Hi, Matthias
Thanks for your reply.
> Although it is quite clear what the patch does, could you write one
> sentence to explain what it does. Maybe explain even which effect it
> has, which error get fixed etc.
Ok, I will do that.
> As we are getting public available boards now, we should take more care
> about fixes. If you have a fix for a commit introduced in an earlier
> version of linux and it should be fixed for this version as well (e.g.
> v4.6 does have the feature but it does not work correctly) then please
> add these two lines before your Signed-off-by:
> Fixes: <commit-hash> ("<commit subject line>")
> Cc: stable@vger.kernel.org # v4.6+
>
> Where v4.6+ stands for the oldest version where this should get fixed.
>
Ok, but the patch hasn't been merged into v4.8 (just in drm-next [1] and
linux-next [2]), how can I mark that?
[1]
https://cgit.freedesktop.org/~airlied/linux/commit/?h=drm-next&id=7216436420414144646f5d8343d061355fd23483
[2]
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=7216436420414144646f5d8343d061355fd23483
> Thanks a lot,
> Matthias
>
> >> Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
> >> ---
> >> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> >> index df33b3c..aa5f20f 100644
> >> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> >> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> >> @@ -123,7 +123,7 @@ static void mtk_od_config(struct mtk_ddp_comp *comp, unsigned int w,
> >> unsigned int bpc)
> >> {
> >> writel(w << 16 | h, comp->regs + DISP_OD_SIZE);
> >> - writel(OD_RELAYMODE, comp->regs + OD_RELAYMODE);
> >> + writel(OD_RELAYMODE, comp->regs + OD_CFG);
> >> mtk_dither_set(comp, bpc, DISP_OD_CFG);
> >> }
> >>
> >
> >
--
Bibby
^ permalink raw reply
* Re: [PATCH v14 2/4] CMDQ: Mediatek CMDQ driver
From: CK Hu @ 2016-09-30 3:06 UTC (permalink / raw)
To: HS Liao
Cc: Rob Herring, Matthias Brugger, Jassi Brar, Daniel Kurtz,
Sascha Hauer, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, srv_heupstream, Sascha Hauer, Philipp Zabel,
Nicolas Boichat, cawa cheng, Bibby Hsieh, YT Shen, Daoyuan Huang,
Damon Chu, Josh-YC Liu, Glory Hung
In-Reply-To: <1473039885-24009-3-git-send-email-hs.liao@mediatek.com>
Hi, HS:
On Mon, 2016-09-05 at 09:44 +0800, HS Liao wrote:
> This patch is first version of Mediatek Command Queue(CMDQ) driver. The
> CMDQ is used to help write registers with critical time limitation,
> such as updating display configuration during the vblank. It controls
> Global Command Engine (GCE) hardware to achieve this requirement.
> Currently, CMDQ only supports display related hardwares, but we expect
> it can be extended to other hardwares for future requirements.
>
> Signed-off-by: HS Liao <hs.liao@mediatek.com>
> Signed-off-by: CK Hu <ck.hu@mediatek.com>
> ---
[snip...]
> +
> +struct cmdq_task {
> + struct cmdq *cmdq;
> + struct list_head list_entry;
> + void *va_base;
> + dma_addr_t pa_base;
> + size_t cmd_buf_size; /* command occupied size */
> + size_t buf_size; /* real buffer size */
> + bool finalized;
> + struct cmdq_thread *thread;
I think thread info could be removed from cmdq_task. Only
cmdq_task_handle_error() and cmdq_task_insert_into_thread() use
task->thread and caller of both function has the thread info. So you
could just pass thread info into these two function and remove thread
info in cmdq_task.
> + struct cmdq_task_cb cb;
I think this callback function is equal to mailbox client tx_done
callback. It's better to use already-defined interface rather than
creating your own.
> +};
> +
[snip...]
> +
> +static int cmdq_suspend(struct device *dev)
> +{
> + struct cmdq *cmdq = dev_get_drvdata(dev);
> + struct cmdq_thread *thread;
> + int i;
> + bool task_running = false;
> +
> + mutex_lock(&cmdq->task_mutex);
> + cmdq->suspended = true;
> + mutex_unlock(&cmdq->task_mutex);
> +
> + for (i = 0; i < ARRAY_SIZE(cmdq->thread); i++) {
> + thread = &cmdq->thread[i];
> + if (!list_empty(&thread->task_busy_list)) {
> + mod_timer(&thread->timeout, jiffies + 1);
> + task_running = true;
> + }
> + }
> +
> + if (task_running) {
> + dev_warn(dev, "exist running task(s) in suspend\n");
> + msleep(20);
Why sleep here? It looks like a recovery but could 20ms recovery
something? I think warning message is enough because you see the warning
message, and you fix the bug, so no need to recovery anything.
> + }
> +
> + clk_unprepare(cmdq->clock);
> + return 0;
> +}
> +
Regards,
CK
^ permalink raw reply
* Re: [PATCH] drm/mediatek: fix a typo
From: Matthias Brugger @ 2016-09-29 8:46 UTC (permalink / raw)
To: CK Hu, Bibby Hsieh
Cc: David Airlie, Daniel Vetter, dri-devel, linux-mediatek,
Yingjoe Chen, Cawa Cheng, Daniel Kurtz, Philipp Zabel, YT Shen,
Thierry Reding, Mao Huang, linux-arm-kernel, linux-kernel,
Sascha Hauer
In-Reply-To: <1475121682.18843.2.camel@mtksdaap41>
On 29/09/16 06:01, CK Hu wrote:
> Acked-by: CK Hu <ck.hu@mediatek.com>
>
> On Thu, 2016-09-29 at 11:22 +0800, Bibby Hsieh wrote:
>> Fix the typo: OD_RELAYMODE->OD_CFG
>>
Although it is quite clear what the patch does, could you write one
sentence to explain what it does. Maybe explain even which effect it
has, which error get fixed etc.
As we are getting public available boards now, we should take more care
about fixes. If you have a fix for a commit introduced in an earlier
version of linux and it should be fixed for this version as well (e.g.
v4.6 does have the feature but it does not work correctly) then please
add these two lines before your Signed-off-by:
Fixes: <commit-hash> ("<commit subject line>")
Cc: stable@vger.kernel.org # v4.6+
Where v4.6+ stands for the oldest version where this should get fixed.
Thanks a lot,
Matthias
>> Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
>> ---
>> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
>> index df33b3c..aa5f20f 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
>> @@ -123,7 +123,7 @@ static void mtk_od_config(struct mtk_ddp_comp *comp, unsigned int w,
>> unsigned int bpc)
>> {
>> writel(w << 16 | h, comp->regs + DISP_OD_SIZE);
>> - writel(OD_RELAYMODE, comp->regs + OD_RELAYMODE);
>> + writel(OD_RELAYMODE, comp->regs + OD_CFG);
>> mtk_dither_set(comp, bpc, DISP_OD_CFG);
>> }
>>
>
>
^ permalink raw reply
* Re: drm/mediatek: fixed the calc method of data rate per lane
From: CK Hu @ 2016-09-29 7:43 UTC (permalink / raw)
To: Jitao Shi
Cc: Mark Rutland, stonea168, dri-devel, Andy Yan, Ajay Kumar,
Vincent Palatin, cawa.cheng, Russell King, devicetree, Pawel Moll,
Ian Campbell, Rob Herring, linux-mediatek, yingjoe.chen,
Matthias Brugger, eddie.huang, linux-arm-kernel, Rahul Sharma,
srv_heupstream, linux-kernel, Sascha Hauer, Kumar Gala
In-Reply-To: <1472191831-7150-1-git-send-email-jitao.shi@mediatek.com>
Hi, Jitao:
Sorry for late reply.
Some comments inline.
On Fri, 2016-08-26 at 14:10 +0800, Jitao Shi wrote:
> Tune dsi frame rate by pixel clock, dsi add some extra signal (i.e. Tlpx,
> Ths-prepare, Ths-zero, Ths-trail,Ths-exit) when enter and exit LP mode, this
> signal will cause h-time larger than normal and reduce FPS.
> Need to multiply a coefficient to offset the extra signal's effect.
> coefficient = ((htotal*bpp/lane_number)+Tlpx+Ths_prep+Ths_zero+Ths_trail+
> Ths_exit)/(htotal*bpp/lane_number))
>
> Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
> ---
> drivers/gpu/drm/mediatek/mtk_dsi.c | 111 ++++++++++++++++++++++--------------
> 1 file changed, 67 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 28b2044..506aa22 100644
[snip...]
>
> -static void dsi_phy_timconfig(struct mtk_dsi *dsi)
> +static void dsi_phy_timconfig(struct mtk_dsi *dsi, u32 phy_timing0,
> + u32 phy_timing1, u32 phy_timing2,
> + u32 phy_timing3)
> {
> - u32 timcon0, timcon1, timcon2, timcon3;
> - unsigned int ui, cycle_time;
> - unsigned int lpx;
> -
> - ui = 1000 / dsi->data_rate + 0x01;
> - cycle_time = 8000 / dsi->data_rate + 0x01;
> - lpx = 5;
> -
> - timcon0 = (8 << 24) | (0xa << 16) | (0x6 << 8) | lpx;
> - timcon1 = (7 << 24) | (5 * lpx << 16) | ((3 * lpx) / 2) << 8 |
> - (4 * lpx);
> - timcon2 = ((NS_TO_CYCLE(0x64, cycle_time) + 0xa) << 24) |
> - (NS_TO_CYCLE(0x150, cycle_time) << 16);
> - timcon3 = (2 * lpx) << 16 | NS_TO_CYCLE(80 + 52 * ui, cycle_time) << 8 |
> - NS_TO_CYCLE(0x40, cycle_time);
> -
> - writel(timcon0, dsi->regs + DSI_PHY_TIMECON0);
> - writel(timcon1, dsi->regs + DSI_PHY_TIMECON1);
> - writel(timcon2, dsi->regs + DSI_PHY_TIMECON2);
> - writel(timcon3, dsi->regs + DSI_PHY_TIMECON3);
> + writel(phy_timing0, dsi->regs + DSI_PHY_TIMECON0);
> + writel(phy_timing1, dsi->regs + DSI_PHY_TIMECON1);
> + writel(phy_timing2, dsi->regs + DSI_PHY_TIMECON2);
> + writel(phy_timing3, dsi->regs + DSI_PHY_TIMECON3);
> }
>
> static void mtk_dsi_enable(struct mtk_dsi *dsi)
> @@ -202,19 +186,57 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
> {
> struct device *dev = dsi->dev;
> int ret;
> + u64 bit_clock, total_bits;
> + u32 htotal, htotal_bits, bit_per_pixel, overhead_cycles, overhead_bits;
> + u32 phy_timing0, phy_timing1, phy_timing2, phy_timing3;
>
> if (++dsi->refcount != 1)
> return 0;
>
> + phy_timing0 = LPX(5) | HS_PRPR(6) | HS_ZERO(10) | HS_TRAIL(8);
> + phy_timing1 = TA_GO(20) | TA_SURE(7) | TA_GET(25) | DA_HS_EXIT(7);
> + phy_timing2 = CLK_ZERO(38) | CLK_TRAIL(22);
> + phy_timing3 = CLK_HS_PRPR(8) | CLK_HS_POST(21) | CLK_HS_EXIT(10);
The original phy_timing2 and phy_timing3 is defined as
timcon2 = ((NS_TO_CYCLE(0x64, cycle_time) + 0xa) << 24) |
(NS_TO_CYCLE(0x150, cycle_time) << 16);
timcon3 = (2 * lpx) << 16 | NS_TO_CYCLE(80 + 52 * ui, cycle_time) << 8
|
NS_TO_CYCLE(0x40, cycle_time);
They are varied by cycle_time. I think you should not use a fixed value
for them.
> +
> + switch (dsi->format) {
> + case MIPI_DSI_FMT_RGB565:
> + bit_per_pixel = 16;
> + break;
> + case MIPI_DSI_FMT_RGB666_PACKED:
> + bit_per_pixel = 18;
> + break;
> + case MIPI_DSI_FMT_RGB666:
> + case MIPI_DSI_FMT_RGB888:
> + default:
> + bit_per_pixel = 24;
> + break;
> + }
> /**
> - * data_rate = (pixel_clock / 1000) * pixel_dipth * mipi_ratio;
> - * pixel_clock unit is Khz, data_rata unit is MHz, so need divide 1000.
> - * mipi_ratio is mipi clk coefficient for balance the pixel clk in mipi.
> - * we set mipi_ratio is 1.05.
> + * data_rate = (pixel_clock) * bit_per_pixel * mipi_ratio / lane_num;
> + * vm.pixelclock is Khz, data_rata unit is Hz, so need to multiply 1000
> + * mipi_ratio is (htotal * byte_per_pixel / lane_num + Tlpx + Ths_prep
> + * + Thstrail + Ths_exit + Ths_zero) /
> + * (htotal * byte_per_pixel /lane_number)
> */
> - dsi->data_rate = dsi->vm.pixelclock * 3 * 21 / (1 * 1000 * 10);
> + bit_clock = dsi->vm.pixelclock * 1000 * bit_per_pixel;
> + htotal = dsi->vm.hactive + dsi->vm.hback_porch + dsi->vm.hfront_porch +
> + dsi->vm.hsync_len;
> + htotal_bits = htotal * bit_per_pixel;
> +
> + /**
> + * overhead = lpx + hs_prepare + hs_zero + hs_trail + hs_exit
> + */
> + overhead_cycles = (phy_timing0 & 0xff) + (phy_timing0 >> 8 & 0xff) +
> + (phy_timing0 >> 16 & 0xff) +
> + (phy_timing0 >> 24 & 0xff) +
> + (phy_timing1 >> 24 & 0xff);
I think phy_timing0 and phy_timing1 is defined as
phy_timing0 = LPX(a) | HS_PRPR(b) | HS_ZERO(c) | HS_TRAIL(d);
phy_timing1 = TA_GO(e) | TA_SURE(f) | TA_GET(g) | DA_HS_EXIT(h);
So, it could be a more simple way to assign overhead_cycles like this
overhead_cycles = a + b + c + d + h;
Because these variable are constant, maybe you can define them as
symbols.
> + overhead_bits = overhead_cycles * dsi->lanes * 8;
> + total_bits = htotal_bits + overhead_bits;
> +
> + dsi->data_rate = DIV_ROUND_UP_ULL(bit_clock * total_bits,
> + htotal_bits * dsi->lanes);
>
> - ret = clk_set_rate(dsi->hs_clk, dsi->data_rate * 1000000);
> + ret = clk_set_rate(dsi->hs_clk, dsi->data_rate);
> if (ret < 0) {
> dev_err(dev, "Failed to set data rate: %d\n", ret);
> goto err_refcount;
> @@ -236,7 +258,8 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
>
> mtk_dsi_enable(dsi);
> mtk_dsi_reset(dsi);
> - dsi_phy_timconfig(dsi);
> + dsi_phy_timconfig(dsi, phy_timing0, phy_timing1, phy_timing2,
> + phy_timing3);
>
> return 0;
>
Regards,
CK
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH 2/2] drm/mediatek: clear IRQ status before enable OVL interrupt
From: CK Hu @ 2016-09-29 5:37 UTC (permalink / raw)
To: Bibby Hsieh
Cc: linux-kernel, Sascha Hauer, Daniel Vetter, Cawa Cheng, dri-devel,
Mao Huang, linux-mediatek, Matthias Brugger, Yingjoe Chen,
linux-arm-kernel
In-Reply-To: <1475119789-64619-3-git-send-email-bibby.hsieh@mediatek.com>
Acked-by: CK Hu <ck.hu@mediatek.com>
On Thu, 2016-09-29 at 11:29 +0800, Bibby Hsieh wrote:
> To make sure that the first vblank IRQ after enabling
> vblank isn't too short or immediate, we have to clear
> the IRQ status before enable OVL interrupt.
>
> Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
> ---
> drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 019b7ca..f75c5b5 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -80,6 +80,7 @@ static void mtk_ovl_enable_vblank(struct mtk_ddp_comp *comp,
> ddp_comp);
>
> priv->crtc = crtc;
> + writel(0x0, comp->regs + DISP_REG_OVL_INTSTA);
> writel_relaxed(OVL_FME_CPL_INT, comp->regs + DISP_REG_OVL_INTEN);
> }
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH 1/2] drm/mediatek: set vblank_disable_allowed to true
From: CK Hu @ 2016-09-29 5:36 UTC (permalink / raw)
To: Bibby Hsieh
Cc: linux-kernel, Sascha Hauer, Daniel Vetter, Cawa Cheng, dri-devel,
Mao Huang, linux-mediatek, Matthias Brugger, Yingjoe Chen,
linux-arm-kernel
In-Reply-To: <1475119789-64619-2-git-send-email-bibby.hsieh@mediatek.com>
Acked-by: CK Hu <ck.hu@mediatek.com>
On Thu, 2016-09-29 at 11:29 +0800, Bibby Hsieh wrote:
> MTK DRM driver didn't set the vblank_disable_allowed to
> true, it cause that the irq_handler is called every
> 16.6 ms (every vblank) when the display didn't be updated.
>
> Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
> ---
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index eebb7d8..941ec5f 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -200,6 +200,7 @@ static int mtk_drm_kms_init(struct drm_device *drm)
> if (ret < 0)
> goto err_component_unbind;
>
> + drm->vblank_disable_allowed = true;
> drm_kms_helper_poll_init(drm);
> drm_mode_config_reset(drm);
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH] drm/mediatek: fix a typo
From: CK Hu @ 2016-09-29 4:01 UTC (permalink / raw)
To: Bibby Hsieh
Cc: linux-kernel, Sascha Hauer, Daniel Vetter, Cawa Cheng, dri-devel,
Mao Huang, linux-mediatek, Matthias Brugger, Yingjoe Chen,
linux-arm-kernel
In-Reply-To: <1475119322-58657-1-git-send-email-bibby.hsieh@mediatek.com>
Acked-by: CK Hu <ck.hu@mediatek.com>
On Thu, 2016-09-29 at 11:22 +0800, Bibby Hsieh wrote:
> Fix the typo: OD_RELAYMODE->OD_CFG
>
> Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
> ---
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> index df33b3c..aa5f20f 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> @@ -123,7 +123,7 @@ static void mtk_od_config(struct mtk_ddp_comp *comp, unsigned int w,
> unsigned int bpc)
> {
> writel(w << 16 | h, comp->regs + DISP_OD_SIZE);
> - writel(OD_RELAYMODE, comp->regs + OD_RELAYMODE);
> + writel(OD_RELAYMODE, comp->regs + OD_CFG);
> mtk_dither_set(comp, bpc, DISP_OD_CFG);
> }
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* [PATCH 2/2] drm/mediatek: clear IRQ status before enable OVL interrupt
From: Bibby Hsieh @ 2016-09-29 3:29 UTC (permalink / raw)
To: David Airlie, Matthias Brugger, Daniel Vetter, dri-devel,
linux-mediatek
Cc: linux-kernel, Cawa Cheng, Mao Huang, Yingjoe Chen, Sascha Hauer,
linux-arm-kernel
In-Reply-To: <1475119789-64619-1-git-send-email-bibby.hsieh@mediatek.com>
To make sure that the first vblank IRQ after enabling
vblank isn't too short or immediate, we have to clear
the IRQ status before enable OVL interrupt.
Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
---
drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
index 019b7ca..f75c5b5 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
@@ -80,6 +80,7 @@ static void mtk_ovl_enable_vblank(struct mtk_ddp_comp *comp,
ddp_comp);
priv->crtc = crtc;
+ writel(0x0, comp->regs + DISP_REG_OVL_INTSTA);
writel_relaxed(OVL_FME_CPL_INT, comp->regs + DISP_REG_OVL_INTEN);
}
--
1.7.9.5
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related
* [PATCH 1/2] drm/mediatek: set vblank_disable_allowed to true
From: Bibby Hsieh @ 2016-09-29 3:29 UTC (permalink / raw)
To: David Airlie, Matthias Brugger, Daniel Vetter, dri-devel,
linux-mediatek
Cc: linux-kernel, Cawa Cheng, Mao Huang, Yingjoe Chen, Sascha Hauer,
linux-arm-kernel
In-Reply-To: <1475119789-64619-1-git-send-email-bibby.hsieh@mediatek.com>
MTK DRM driver didn't set the vblank_disable_allowed to
true, it cause that the irq_handler is called every
16.6 ms (every vblank) when the display didn't be updated.
Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
---
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index eebb7d8..941ec5f 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -200,6 +200,7 @@ static int mtk_drm_kms_init(struct drm_device *drm)
if (ret < 0)
goto err_component_unbind;
+ drm->vblank_disable_allowed = true;
drm_kms_helper_poll_init(drm);
drm_mode_config_reset(drm);
--
1.7.9.5
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related
* [PATCH 0/2] fix issue: vblank interrupts are never disabled
From: Bibby Hsieh @ 2016-09-29 3:29 UTC (permalink / raw)
To: David Airlie, Matthias Brugger, Daniel Vetter, dri-devel,
linux-mediatek
Cc: linux-kernel, Cawa Cheng, Mao Huang, Yingjoe Chen, Sascha Hauer,
linux-arm-kernel
Clean the interrupt status before enable interrupt
and set the vblank_disable_allowed to fix the issue.
Bibby Hsieh (2):
drm/mediatek: set vblank_disable_allowed to true
drm/mediatek: clear IRQ status before enable OVL interrupt
drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 1 +
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 1 +
2 files changed, 2 insertions(+)
--
1.7.9.5
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* [PATCH] drm/mediatek: fix a typo
From: Bibby Hsieh @ 2016-09-29 3:22 UTC (permalink / raw)
To: David Airlie, Matthias Brugger, Daniel Vetter, dri-devel,
linux-mediatek
Cc: linux-kernel, Cawa Cheng, Mao Huang, Yingjoe Chen, Sascha Hauer,
linux-arm-kernel
Fix the typo: OD_RELAYMODE->OD_CFG
Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
---
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
index df33b3c..aa5f20f 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -123,7 +123,7 @@ static void mtk_od_config(struct mtk_ddp_comp *comp, unsigned int w,
unsigned int bpc)
{
writel(w << 16 | h, comp->regs + DISP_OD_SIZE);
- writel(OD_RELAYMODE, comp->regs + OD_RELAYMODE);
+ writel(OD_RELAYMODE, comp->regs + OD_CFG);
mtk_dither_set(comp, bpc, DISP_OD_CFG);
}
--
1.7.9.5
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related
* [PATCH v5 3/3] drm/mediatek: modify the factor to make the pll_rate set in the 1G-2G range
From: Bibby Hsieh @ 2016-09-29 3:02 UTC (permalink / raw)
To: David Airlie, Matthias Brugger, Daniel Vetter, dri-devel,
linux-mediatek
Cc: Yingjoe Chen, Cawa Cheng, Daniel Kurtz, Bibby Hsieh,
Philipp Zabel, YT Shen, Thierry Reding, CK Hu, Mao Huang,
linux-arm-kernel, linux-kernel, Sascha Hauer, Junzhi Zhao
In-Reply-To: <1475118135-56780-1-git-send-email-bibby.hsieh@mediatek.com>
From: Junzhi Zhao <junzhi.zhao@mediatek.com>
Currently, the code sets the "pll" to the desired multiple
of the pixel clock manully(4*3m 8*3,etc). The valid range
of the pll is 1G-2G, however, when the pixel clock is bigger
than 167MHz, the "pll" will be set to a invalid value( > 2G),
then the "pll" will be 2GHz, thus the pixel clock will be in
correct. Change the factor to make the "pll" be set in the
(1G, 2G) range.
Signed-off-by: Junzhi Zhao <junzhi.zhao@mediatek.com>
Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
---
drivers/gpu/drm/mediatek/mtk_dpi.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
index 0186e50..90fb831 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
@@ -432,11 +432,16 @@ static int mtk_dpi_set_display_mode(struct mtk_dpi *dpi,
unsigned long pll_rate;
unsigned int factor;
+ /* let pll_rate can fix the valid range of tvdpll (1G~2GHz) */
pix_rate = 1000UL * mode->clock;
- if (mode->clock <= 74000)
+ if (mode->clock <= 27000)
+ factor = 16 * 3;
+ else if (mode->clock <= 84000)
factor = 8 * 3;
- else
+ else if (mode->clock <= 167000)
factor = 4 * 3;
+ else
+ factor = 2 * 3;
pll_rate = pix_rate * factor;
dev_dbg(dpi->dev, "Want PLL %lu Hz, pixel clock %lu Hz\n",
--
1.7.9.5
^ permalink raw reply related
* [PATCH v5 2/3] drm/mediatek: enhance the HDMI driving current
From: Bibby Hsieh @ 2016-09-29 3:02 UTC (permalink / raw)
To: David Airlie, Matthias Brugger, Daniel Vetter, dri-devel,
linux-mediatek
Cc: Yingjoe Chen, Cawa Cheng, Daniel Kurtz, Bibby Hsieh,
Philipp Zabel, YT Shen, Thierry Reding, CK Hu, Mao Huang,
linux-arm-kernel, linux-kernel, Sascha Hauer, Junzhi Zhao
In-Reply-To: <1475118135-56780-1-git-send-email-bibby.hsieh@mediatek.com>
From: Junzhi Zhao <junzhi.zhao@mediatek.com>
In order to improve 4K resolution performance,
we have to enhance the HDMI driving current
when clock rate is greater than 165MHz.
Signed-off-by: Junzhi Zhao <junzhi.zhao@mediatek.com>
Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
---
drivers/gpu/drm/mediatek/mtk_mt8173_hdmi_phy.c | 42 +++++++++++++++++-------
1 file changed, 30 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_mt8173_hdmi_phy.c b/drivers/gpu/drm/mediatek/mtk_mt8173_hdmi_phy.c
index 8a24754..51cb9cf 100644
--- a/drivers/gpu/drm/mediatek/mtk_mt8173_hdmi_phy.c
+++ b/drivers/gpu/drm/mediatek/mtk_mt8173_hdmi_phy.c
@@ -265,6 +265,9 @@ static int mtk_hdmi_pll_set_rate(struct clk_hw *hw, unsigned long rate,
struct mtk_hdmi_phy *hdmi_phy = to_mtk_hdmi_phy(hw);
unsigned int pre_div;
unsigned int div;
+ unsigned int pre_ibias;
+ unsigned int hdmi_ibias;
+ unsigned int imp_en;
dev_dbg(hdmi_phy->dev, "%s: %lu Hz, parent: %lu Hz\n", __func__,
rate, parent_rate);
@@ -298,18 +301,31 @@ static int mtk_hdmi_pll_set_rate(struct clk_hw *hw, unsigned long rate,
(0x1 << PLL_BR_SHIFT),
RG_HDMITX_PLL_BP | RG_HDMITX_PLL_BC |
RG_HDMITX_PLL_BR);
- mtk_hdmi_phy_clear_bits(hdmi_phy, HDMI_CON3, RG_HDMITX_PRD_IMP_EN);
+ if (rate < 165000000) {
+ mtk_hdmi_phy_clear_bits(hdmi_phy, HDMI_CON3,
+ RG_HDMITX_PRD_IMP_EN);
+ pre_ibias = 0x3;
+ imp_en = 0x0;
+ hdmi_ibias = hdmi_phy->ibias;
+ } else {
+ mtk_hdmi_phy_set_bits(hdmi_phy, HDMI_CON3,
+ RG_HDMITX_PRD_IMP_EN);
+ pre_ibias = 0x6;
+ imp_en = 0xf;
+ hdmi_ibias = hdmi_phy->ibias_up;
+ }
mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON4,
- (0x3 << PRD_IBIAS_CLK_SHIFT) |
- (0x3 << PRD_IBIAS_D2_SHIFT) |
- (0x3 << PRD_IBIAS_D1_SHIFT) |
- (0x3 << PRD_IBIAS_D0_SHIFT),
+ (pre_ibias << PRD_IBIAS_CLK_SHIFT) |
+ (pre_ibias << PRD_IBIAS_D2_SHIFT) |
+ (pre_ibias << PRD_IBIAS_D1_SHIFT) |
+ (pre_ibias << PRD_IBIAS_D0_SHIFT),
RG_HDMITX_PRD_IBIAS_CLK |
RG_HDMITX_PRD_IBIAS_D2 |
RG_HDMITX_PRD_IBIAS_D1 |
RG_HDMITX_PRD_IBIAS_D0);
mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON3,
- (0x0 << DRV_IMP_EN_SHIFT), RG_HDMITX_DRV_IMP_EN);
+ (imp_en << DRV_IMP_EN_SHIFT),
+ RG_HDMITX_DRV_IMP_EN);
mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON6,
(hdmi_phy->drv_imp_clk << DRV_IMP_CLK_SHIFT) |
(hdmi_phy->drv_imp_d2 << DRV_IMP_D2_SHIFT) |
@@ -318,12 +334,14 @@ static int mtk_hdmi_pll_set_rate(struct clk_hw *hw, unsigned long rate,
RG_HDMITX_DRV_IMP_CLK | RG_HDMITX_DRV_IMP_D2 |
RG_HDMITX_DRV_IMP_D1 | RG_HDMITX_DRV_IMP_D0);
mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON5,
- (hdmi_phy->ibias << DRV_IBIAS_CLK_SHIFT) |
- (hdmi_phy->ibias << DRV_IBIAS_D2_SHIFT) |
- (hdmi_phy->ibias << DRV_IBIAS_D1_SHIFT) |
- (hdmi_phy->ibias << DRV_IBIAS_D0_SHIFT),
- RG_HDMITX_DRV_IBIAS_CLK | RG_HDMITX_DRV_IBIAS_D2 |
- RG_HDMITX_DRV_IBIAS_D1 | RG_HDMITX_DRV_IBIAS_D0);
+ (hdmi_ibias << DRV_IBIAS_CLK_SHIFT) |
+ (hdmi_ibias << DRV_IBIAS_D2_SHIFT) |
+ (hdmi_ibias << DRV_IBIAS_D1_SHIFT) |
+ (hdmi_ibias << DRV_IBIAS_D0_SHIFT),
+ RG_HDMITX_DRV_IBIAS_CLK |
+ RG_HDMITX_DRV_IBIAS_D2 |
+ RG_HDMITX_DRV_IBIAS_D1 |
+ RG_HDMITX_DRV_IBIAS_D0);
return 0;
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH v5 1/3] drm/mediatek: do mtk_hdmi_send_infoframe after HDMI clock enable
From: Bibby Hsieh @ 2016-09-29 3:02 UTC (permalink / raw)
To: David Airlie, Matthias Brugger, Daniel Vetter, dri-devel,
linux-mediatek
Cc: Yingjoe Chen, Cawa Cheng, Daniel Kurtz, Bibby Hsieh,
Philipp Zabel, YT Shen, Thierry Reding, CK Hu, Mao Huang,
linux-arm-kernel, linux-kernel, Sascha Hauer, Junzhi Zhao
In-Reply-To: <1475118135-56780-1-git-send-email-bibby.hsieh@mediatek.com>
From: Junzhi Zhao <junzhi.zhao@mediatek.com>
The mtk_hdmi_send_infoframe have to
be run after PLL and PIXEL clock of HDMI enable.
Make sure that HDMI inforframes can be sent
successfully.
Signed-off-by: Junzhi Zhao <junzhi.zhao@mediatek.com>
Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
---
drivers/gpu/drm/mediatek/mtk_hdmi.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
index 334562d..875b045 100644
--- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
+++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
@@ -1133,12 +1133,6 @@ static int mtk_hdmi_output_set_display_mode(struct mtk_hdmi *hdmi,
phy_power_on(hdmi->phy);
mtk_hdmi_aud_output_config(hdmi, mode);
- mtk_hdmi_setup_audio_infoframe(hdmi);
- mtk_hdmi_setup_avi_infoframe(hdmi, mode);
- mtk_hdmi_setup_spd_infoframe(hdmi, "mediatek", "On-chip HDMI");
- if (mode->flags & DRM_MODE_FLAG_3D_MASK)
- mtk_hdmi_setup_vendor_specific_infoframe(hdmi, mode);
-
mtk_hdmi_hw_vid_black(hdmi, false);
mtk_hdmi_hw_aud_unmute(hdmi);
mtk_hdmi_hw_send_av_unmute(hdmi);
@@ -1401,6 +1395,16 @@ static void mtk_hdmi_bridge_pre_enable(struct drm_bridge *bridge)
hdmi->powered = true;
}
+static void mtk_hdmi_send_infoframe(struct mtk_hdmi *hdmi,
+ struct drm_display_mode *mode)
+{
+ mtk_hdmi_setup_audio_infoframe(hdmi);
+ mtk_hdmi_setup_avi_infoframe(hdmi, mode);
+ mtk_hdmi_setup_spd_infoframe(hdmi, "mediatek", "On-chip HDMI");
+ if (mode->flags & DRM_MODE_FLAG_3D_MASK)
+ mtk_hdmi_setup_vendor_specific_infoframe(hdmi, mode);
+}
+
static void mtk_hdmi_bridge_enable(struct drm_bridge *bridge)
{
struct mtk_hdmi *hdmi = hdmi_ctx_from_bridge(bridge);
@@ -1409,6 +1413,7 @@ static void mtk_hdmi_bridge_enable(struct drm_bridge *bridge)
clk_prepare_enable(hdmi->clk[MTK_HDMI_CLK_HDMI_PLL]);
clk_prepare_enable(hdmi->clk[MTK_HDMI_CLK_HDMI_PIXEL]);
phy_power_on(hdmi->phy);
+ mtk_hdmi_send_infoframe(hdmi, &hdmi->mode);
hdmi->enabled = true;
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH v5 0/3] MT8173 HDMI 4K support
From: Bibby Hsieh @ 2016-09-29 3:02 UTC (permalink / raw)
To: David Airlie, Matthias Brugger, Daniel Vetter, dri-devel,
linux-mediatek
Cc: Yingjoe Chen, Cawa Cheng, Daniel Kurtz, Bibby Hsieh,
Philipp Zabel, YT Shen, Thierry Reding, CK Hu, Mao Huang,
linux-arm-kernel, linux-kernel, Sascha Hauer
This is MT8173 HDMI 4K support PATCH v5, based on 4.8-rc1.
In order to support HDMI 4K on MT8173,
we have to make some modifications.
1) Make sure that mtk_hdmi_send_infoframe is sent successfully.
2) Enhance the HDMI driving current to improve performance.
3) Make sure that pixel clock is 297MHz when resolution is 4K.
Changes since v4:
- Update commit message and patch title.
Changes since v3:
- Rebase to 4.8-rc1.
- The valid range of tvdpll is 1G to 2G Hz, so, we Change the
if statement of mode->clock to fit that and add a comment.
Changes since v2:
- Remove the change about preparation for MT2701 support.
Changes since v1:
- According to the suggestion from philipp, We use the new
dpi0_sel rate set method.
- calls clk_set_rate to set the dpi0_sel according to the
pixel clock.
- Remove the direct access to all the intermediate clock part.
- Remove the intermediate tvdpll_d* clocks in dts.
- According to suggestion from CK, we rename the clock parse
function and remove it from mtk_dpi_conf struct.
- Merges the hdmi Pll set rate for pixel clock greater than
165MHz and smaller parts.
The PATCH depends on the following patch:
https://patchwork.kernel.org/patch/9262575/
(arm64: dts: mt8173: add mmsel clocks for 4K support)
Junzhi Zhao (3):
drm/mediatek: do mtk_hdmi_send_infoframe after HDMI clock enable
drm/mediatek: enhance the HDMI driving current
drm/mediatek: modify the factor to make the pll_rate set in the 1G-2G
range
drivers/gpu/drm/mediatek/mtk_dpi.c | 9 +++--
drivers/gpu/drm/mediatek/mtk_hdmi.c | 17 ++++++----
drivers/gpu/drm/mediatek/mtk_mt8173_hdmi_phy.c | 42 +++++++++++++++++-------
3 files changed, 48 insertions(+), 20 deletions(-)
--
1.7.9.5
^ permalink raw reply
* RE: [PATCH net-next 0/2] net: ethernet: mediatek: some bug fixes for PDAM and HW LRO
From: Nelson Chang @ 2016-09-27 17:16 UTC (permalink / raw)
To: davem; +Cc: john, nbd, netdev, linux-mediatek, nelsonch.tw
Got it. I'll notice that for later patches.
Thanks David.
-----Original Message-----
From: David Miller [mailto:davem@davemloft.net]
Sent: Tuesday, September 27, 2016 9:42 PM
To: Nelson Chang (張家祥)
Cc: john@phrozen.org; nbd@openwrt.org; netdev@vger.kernel.org;
linux-mediatek@lists.infradead.org; nelsonch.tw@gmail.com
Subject: Re: [PATCH net-next 0/2] net: ethernet: mediatek: some bug
fixes for PDAM and HW LRO
From: Nelson Chang <nelson.chang@mediatek.com>
Date: Mon, 26 Sep 2016 14:33:48 +0800
> 1) Add to stop PDMA while stopping the frame engine
> 2) Modify the register settings for LRO relinquishments
> 3) Jump out from the waiting loop while LRO relinquishments are done
Series applied, but like Sergei I think you should have split patch
#2 into two separate patches.
You even list the changes individually here in your header posting.
^ permalink raw reply
* Re: [PATCH 2/2] drm/rockchip: mark symbols static where possible
From: Sean Paul @ 2016-09-27 16:34 UTC (permalink / raw)
To: Baoyou Xie
Cc: Arnd Bergmann, linux-rockchip, tang.qiang007, Baoyou Xie,
Linux Kernel Mailing List, dri-devel, Matthias Brugger,
linux-mediatek, han.fei, Linux ARM Kernel
In-Reply-To: <1474789388-3284-1-git-send-email-baoyou.xie@linaro.org>
On Sun, Sep 25, 2016 at 3:43 AM, Baoyou Xie <baoyou.xie@linaro.org> wrote:
> We get 2 warnings when building kernel with W=1:
> drivers/gpu/drm/rockchip/rockchip_drm_drv.c:309:6: warning: no previous prototype for 'rockchip_drm_fb_suspend' [-Wmissing-prototypes]
> drivers/gpu/drm/rockchip/rockchip_drm_drv.c:318:6: warning: no previous prototype for 'rockchip_drm_fb_resume' [-Wmissing-prototypes]
>
> In fact, these functions are only used in the file in which they are
> declared and don't need a declaration, but can be made static.
> So this patch marks these functions with 'static'.
>
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
Applied to -misc, thanks.
Sean
> ---
> drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> index 76eaf1d..38c3be5 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> @@ -309,7 +309,7 @@ static struct drm_driver rockchip_drm_driver = {
> };
>
> #ifdef CONFIG_PM_SLEEP
> -void rockchip_drm_fb_suspend(struct drm_device *drm)
> +static void rockchip_drm_fb_suspend(struct drm_device *drm)
> {
> struct rockchip_drm_private *priv = drm->dev_private;
>
> @@ -318,7 +318,7 @@ void rockchip_drm_fb_suspend(struct drm_device *drm)
> console_unlock();
> }
>
> -void rockchip_drm_fb_resume(struct drm_device *drm)
> +static void rockchip_drm_fb_resume(struct drm_device *drm)
> {
> struct rockchip_drm_private *priv = drm->dev_private;
>
> --
> 2.7.4
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox