Re: [RFC PATCH v2 13/17] gpu: host1x: Reset max value when freeing a syncpoint

2020-09-17 Thread Dmitry Osipenko
16.09.2020 23:43, Mikko Perttunen пишет:
...
>> Please note that the sync point state actually needs to be completely
>> reset at the sync point request-time because both downstream fastboot
>> and upstream u-boot [1] are needlessly enabling display VBLANK interrupt
>> that continuously increments sync point #26 during of kernel boot until
>> display controller is reset.
>>
>> [1]
>> https://github.com/u-boot/u-boot/blob/master/drivers/video/tegra.c#L155
>>
>> Hence once sync point #26 is requested, it will have a dirty state. So
>> far this doesn't have any visible effect because sync points aren't used
>> much.
>>
> 
> Maybe we can instead reserve syncpoints that might be used by the boot
> chain, and only allow allocating them once the display driver has acked
> that the syncpoint will no longer be incremented? That way if the
> display driver is disabled for some reason we'll still be fine.

sounds good

> Looking at the downstream driver, it (still, on new chips..) reserves
> the following syncpoints:
> 
> - 10 (AVP)
> - 22 (3D)
> - 26 (VBLANK0)
> - 27 (VBLANK1)
> 
> and says that this applies to T20, T30, T114 and T148.
> 
> I suppose if you haven't observed this happening to other syncpoints
> than 26, then reserving 26 would probably be enough.

I only saw SP 26 being used by the DC, but perhaps that may vary from
device to device and SP 27 could actually be used in a wild as well.

I think the AVP SP should only relate to the AVP-firmware that upstream
doesn't support, so we can ignore its reservation.

I've no idea what may use the 3D SP.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC PATCH v2 13/17] gpu: host1x: Reset max value when freeing a syncpoint

2020-09-17 Thread Dmitry Osipenko
05.09.2020 13:34, Mikko Perttunen пишет:
> With job recovery becoming optional, syncpoints may have a mismatch
> between their value and max value when freed. As such, when freeing,
> set the max value to the current value of the syncpoint so that it
> is in a sane state for the next user.
> 
> Signed-off-by: Mikko Perttunen 
> ---
>  drivers/gpu/host1x/syncpt.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/host1x/syncpt.c b/drivers/gpu/host1x/syncpt.c
> index 2fad8b2a55cc..82ecb4ac387e 100644
> --- a/drivers/gpu/host1x/syncpt.c
> +++ b/drivers/gpu/host1x/syncpt.c
> @@ -385,6 +385,7 @@ static void syncpt_release(struct kref *ref)
>  {
>   struct host1x_syncpt *sp = container_of(ref, struct host1x_syncpt, ref);
>  
> + atomic_set(>max_val, host1x_syncpt_read_min(sp));
>   sp->locked = false;
>  
>   mutex_lock(>host->syncpt_mutex);
> 

Please note that the sync point state actually needs to be completely
reset at the sync point request-time because both downstream fastboot
and upstream u-boot [1] are needlessly enabling display VBLANK interrupt
that continuously increments sync point #26 during of kernel boot until
display controller is reset.

[1] https://github.com/u-boot/u-boot/blob/master/drivers/video/tegra.c#L155

Hence once sync point #26 is requested, it will have a dirty state. So
far this doesn't have any visible effect because sync points aren't used
much.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC PATCH v2 10/17] WIP: gpu: host1x: Add no-recovery mode

2020-09-14 Thread Dmitry Osipenko
05.09.2020 13:34, Mikko Perttunen пишет:
> + } else {
> + struct host1x_job *failed_job = job;
> +
> + host1x_job_dump(dev, job);
> +
> + host1x_syncpt_set_locked(job->syncpt);
> + failed_job->cancelled = true;
> +
> + list_for_each_entry_continue(job, >sync_queue, list) {
> + unsigned int i;
> +
> + if (job->syncpt != failed_job->syncpt)
> + continue;
> +
> + for (i = 0; i < job->num_slots; i++) {
> + unsigned int slot = (job->first_get/8 + i) %
> + HOST1X_PUSHBUFFER_SLOTS;
> + u32 *mapped = cdma->push_buffer.mapped;
> +
> + mapped[2*slot+0] = 0x1bad;
> + mapped[2*slot+1] = 0x1bad;

The 0x1bad is a valid memory address on Tegra20.

The 0x6000 is invalid phys address for all hardware generations.
It's used by grate-kernel [1] and VDE driver [2]. Note that the 0x6 <<
28 is also invalid Host1x opcode, while 0x1 should break CDMA parser
during of PB debug-dumping.

[1]
https://github.com/grate-driver/linux/blob/master/drivers/gpu/drm/tegra/gem.h#L16

[2]
https://elixir.bootlin.com/linux/v5.9-rc4/source/drivers/staging/media/tegra-vde/iommu.c#L99

The VDE driver reserves the trapping IOVA addresses, I assume the Host1x
driver should do the same.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC PATCH v2 10/17] WIP: gpu: host1x: Add no-recovery mode

2020-09-14 Thread Dmitry Osipenko
12.09.2020 16:31, Mikko Perttunen пишет:
...
>> I'm now taking a closer look at this patch and it raises some more
>> questions, like for example by looking at the "On job timeout, we stop
>> the channel, NOP all future jobs on the channel using the same syncpoint
>> ..." through the prism of grate-kernel experience, I'm not sure how it
>> could co-exist with the drm-scheduler and why it's needed at all. But I
>> think we need a feature-complete version (at least a rough version), so
>> that we could start the testing, and then it should be easier to review
>> and discuss such things.
>>
> 
> The reason this is needed is that if a job times out and we don't do its
> syncpoint increments on the CPU, then a successive job incrementing that
> same syncpoint would cause fences to signal incorrectly. The job that
> was supposed to signal those fences didn't actually run; and any data
> those fences were protecting would still be garbage.

I'll need to re-read the previous discussion because IIRC, I was
suggesting that once job is hung, all jobs should be removed from
queue/PB and re-submitted, then the re-submitted jobs will use the
new/updated sync point base.

And we probably should need another drm_tegra_submit_cmd type that waits
for a relative sync point increment. The
drm_tegra_submit_cmd_wait_syncpt uses absolute sync point value and it
shouldn't be used for sync point increments that are internal to a job
because it complicates the recovery.

All waits that are internal to a job should only wait for relative sync
point increments.

In the grate-kernel every job uses unique-and-clean sync point (which is
also internal to the kernel driver) and a relative wait [1] is used for
the job's internal sync point increments [2][3][4], and thus, kernel
driver simply jumps over a hung job by updating DMAGET to point at the
start of a next job.

[1]
https://github.com/grate-driver/linux/blob/master/drivers/gpu/drm/tegra/uapi/patching.c#L367

[2]
https://github.com/grate-driver/xf86-video-opentegra/blob/master/src/gpu/gr3d.c#L486
[3]
https://github.com/grate-driver/xf86-video-opentegra/blob/master/src/exa/copy_2d.c#L389
[4]
https://github.com/grate-driver/xf86-video-opentegra/blob/master/src/gpu/tegra_stream_v2.c#L536
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC PATCH v2 10/17] WIP: gpu: host1x: Add no-recovery mode

2020-09-14 Thread Dmitry Osipenko
13.09.2020 12:51, Mikko Perttunen пишет:
...
>> All waits that are internal to a job should only wait for relative sync
>> point increments. >
>> In the grate-kernel every job uses unique-and-clean sync point (which is
>> also internal to the kernel driver) and a relative wait [1] is used for
>> the job's internal sync point increments [2][3][4], and thus, kernel
>> driver simply jumps over a hung job by updating DMAGET to point at the
>> start of a next job.
> 
> Issues I have with this approach:
> 
> * Both this and my approach have the requirement for userspace, that if
> a job hangs, the userspace must ensure all external waiters have timed
> out / been stopped before the syncpoint can be freed, as if the
> syncpoint gets reused before then, false waiter completions can happen.
> 
> So freeing the syncpoint must be exposed to userspace. The kernel cannot
> do this since there may be waiters that the kernel is not aware of. My
> proposal only has one syncpoint, which I feel makes this part simpler, too.
> 
> * I believe this proposal requires allocating a syncpoint for each
> externally visible syncpoint increment that the job does. This can use
> up quite a few syncpoints, and it makes syncpoints a dynamically
> allocated resource with unbounded allocation latency. This is a problem
> for safety-related systems.

Maybe we could have a special type of a "shared" sync point that is
allocated per-hardware engine? Then shared SP won't be a scarce resource
and job won't depend on it. The kernel or userspace driver may take care
of recovering the counter value of a shared SP when job hangs or do
whatever else is needed without affecting the job's sync point.

Primarily I'm not feeling very happy about retaining the job's sync
point recovery code because it was broken the last time I touched it and
grate-kernel works fine without it.

> * If a job fails on a "virtual channel" (userctx), I think it's a
> reasonable expectation that further jobs on that "virtual channel" will
> not execute, and I think implementing that model is simpler than doing
> recovery.

Couldn't jobs just use explicit fencing? Then a second job won't be
executed if first job hangs and explicit dependency is expressed. I'm
not sure that concept of a "virtual channel" is applicable to drm-scheduler.

I'll need to see a full-featured driver implementation and the test
cases that cover all the problems that you're worried about because I'm
not aware about all the T124+ needs and seeing code should help. Maybe
in the end yours approach will be the best, but for now it's not clear :)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC PATCH v2 17/17] WIP: drm/tegra: Implement new UAPI

2020-09-14 Thread Dmitry Osipenko
11.09.2020 12:59, Mikko Perttunen пишет:
> On 9/11/20 12:57 AM, Dmitry Osipenko wrote:
>> 09.09.2020 11:36, Mikko Perttunen пишет:
>> ...
>>>>
>>>> Does it make sense to have timeout in microseconds?
>>>>
>>>
>>> Not sure, but better have it a bit more fine-grained rather than
>>> coarse-grained. This still gives a maximum timeout of 71 minutes so I
>>> don't think it has any negatives compared to milliseconds.
>>
>> If there is no good reason to use microseconds right now, then should be
>> better to default to milliseconds, IMO. It shouldn't be a problem to
>> extend the IOCLT with a microseconds entry, if ever be needed.
>>
>> {
>> __u32 timeout_ms;
>> ...
>> __u32 timeout_us;
>> }
>>
>> timeout = timeout_ms + 1000 * timeout_us;
>>
>> There shouldn't be a need for a long timeouts, since a job that takes
>> over 100ms is probably too unpractical. It also should be possible to
>> detect a progressing job and then defer timeout in the driver. At least
>> this is what other drivers do, like etnaviv driver for example:
>>
>> https://elixir.bootlin.com/linux/v5.9-rc4/source/drivers/gpu/drm/etnaviv/etnaviv_sched.c#L107
>>
>>
> 
> I still don't quite understand why it's better to default to
> milliseconds? As you say, there is no need to have a long timeout, and
> if we go microseconds now, then there wouldn't be a need to extend in
> the future.

It will nicer to avoid unnecessary unit-conversions in the code in order
to keep it cleaner.

I'm now also a bit dubious about that the timeout field of the submit
IOCTL will be in the final UAPI version because it should become
obsolete once drm-scheduler will be hooked up, since the hung-check
timeout will be specified per-hardware engine within the kernel driver
and there won't be much use for the user-defined timeout.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC PATCH v2 10/17] WIP: gpu: host1x: Add no-recovery mode

2020-09-14 Thread Dmitry Osipenko
12.09.2020 01:11, Mikko Perttunen пишет:
> On 9/11/20 7:40 PM, Dmitry Osipenko wrote:
>> 05.09.2020 13:34, Mikko Perttunen пишет:
>>> +    } else {
>>> +    struct host1x_job *failed_job = job;
>>> +
>>> +    host1x_job_dump(dev, job);
>>> +
>>> +    host1x_syncpt_set_locked(job->syncpt);
>>> +    failed_job->cancelled = true;
>>> +
>>> +    list_for_each_entry_continue(job, >sync_queue, list) {
>>> +    unsigned int i;
>>> +
>>> +    if (job->syncpt != failed_job->syncpt)
>>> +    continue;
>>> +
>>> +    for (i = 0; i < job->num_slots; i++) {
>>> +    unsigned int slot = (job->first_get/8 + i) %
>>> +    HOST1X_PUSHBUFFER_SLOTS;
>>> +    u32 *mapped = cdma->push_buffer.mapped;
>>> +
>>> +    mapped[2*slot+0] = 0x1bad;
>>> +    mapped[2*slot+1] = 0x1bad;
>>
>> The 0x1bad is a valid memory address on Tegra20.
>>
>> The 0x6000 is invalid phys address for all hardware generations.
>> It's used by grate-kernel [1] and VDE driver [2]. Note that the 0x6 <<
>> 28 is also invalid Host1x opcode, while 0x1 should break CDMA parser
>> during of PB debug-dumping.
>>
>> [1]
>> https://github.com/grate-driver/linux/blob/master/drivers/gpu/drm/tegra/gem.h#L16
>>
>>
>> [2]
>> https://elixir.bootlin.com/linux/v5.9-rc4/source/drivers/staging/media/tegra-vde/iommu.c#L99
>>
>>
>> The VDE driver reserves the trapping IOVA addresses, I assume the Host1x
>> driver should do the same.
>>
> 
> The 0x1bad's are not intended to be memory addresses, they are NOOP
> opcodes (INCR of 0 words to offset 0xbad). I'll fix this to use proper
> functions to construct the opcodes and add some comments. These need to
> be NOOP opcodes so the command parser skips over these "cancelled" jobs
> when the channel is resumed.
> 
> BTW, 0x6000 is valid on Tegra194 and later.

At a quick glance it looked like a memory address :)

I'm now taking a closer look at this patch and it raises some more
questions, like for example by looking at the "On job timeout, we stop
the channel, NOP all future jobs on the channel using the same syncpoint
..." through the prism of grate-kernel experience, I'm not sure how it
could co-exist with the drm-scheduler and why it's needed at all. But I
think we need a feature-complete version (at least a rough version), so
that we could start the testing, and then it should be easier to review
and discuss such things.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC PATCH v2 09/17] gpu: host1x: DMA fences and userspace fence creation

2020-09-11 Thread Dmitry Osipenko
05.09.2020 13:34, Mikko Perttunen пишет:
...
>  
> +static void action_signal_fence(struct host1x_waitlist *waiter)
> +{
> + struct host1x_syncpt_fence *f = waiter->data;
> +
> + host1x_fence_signal(f);
> +}
> +
>  typedef void (*action_handler)(struct host1x_waitlist *waiter);
>  
>  static const action_handler action_handlers[HOST1X_INTR_ACTION_COUNT] = {
>   action_submit_complete,
>   action_wakeup,
>   action_wakeup_interruptible,
> + action_signal_fence,
>  };

My expectation is that we should remove the host1x-waiter entirely. It
comes from 2011/2012 era of the host1x driver and now duplicates
functionality provided by the dma-fence and drm-scheduler. Perhaps it
could be okay to re-use existing code for the starter, but this is
something to keep in mind that it may be better not to put much effort
into the older code.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC PATCH v2 17/17] WIP: drm/tegra: Implement new UAPI

2020-09-11 Thread Dmitry Osipenko
09.09.2020 11:36, Mikko Perttunen пишет:
...
>>
>> Does it make sense to have timeout in microseconds?
>>
> 
> Not sure, but better have it a bit more fine-grained rather than
> coarse-grained. This still gives a maximum timeout of 71 minutes so I
> don't think it has any negatives compared to milliseconds.

If there is no good reason to use microseconds right now, then should be
better to default to milliseconds, IMO. It shouldn't be a problem to
extend the IOCLT with a microseconds entry, if ever be needed.

{
__u32 timeout_ms;
...
__u32 timeout_us;
}

timeout = timeout_ms + 1000 * timeout_us;

There shouldn't be a need for a long timeouts, since a job that takes
over 100ms is probably too unpractical. It also should be possible to
detect a progressing job and then defer timeout in the driver. At least
this is what other drivers do, like etnaviv driver for example:

https://elixir.bootlin.com/linux/v5.9-rc4/source/drivers/gpu/drm/etnaviv/etnaviv_sched.c#L107
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: DRM_TEGRA_SUBMIT_BUF_WRITE_RELOC

2020-09-11 Thread Dmitry Osipenko
09.09.2020 11:10, Mikko Perttunen пишет:
> On 9/9/20 2:45 AM, Dmitry Osipenko wrote:
>> 05.09.2020 13:34, Mikko Perttunen пишет:
>> ...
>>> +/* Submission */
>>> +
>>> +/** Patch address of the specified mapping in the submitted gather. */
>>> +#define DRM_TEGRA_SUBMIT_BUF_WRITE_RELOC    (1<<0)
>>
>> Shouldn't the kernel driver be aware about what relocations need to be
>> patched? Could you please explain the purpose of this flag?
>>
> 
> Sure, the kernel knows if it returned the IOVA to the user or not, so we
> could remove this flag and determine it implicitly. I don't think there
> is much harm in it though; if we have the flag an application can decide
> to ignore the iova field and just pass WRITE_RELOC always, and it's not
> really any extra code on kernel side.

Sounds like there is no real practical use for this flag other than for
testing purposes, correct?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC PATCH v2 17/17] WIP: drm/tegra: Implement new UAPI

2020-09-11 Thread Dmitry Osipenko
09.09.2020 11:19, Mikko Perttunen пишет:
...
>>> +    if (!job_data->used_mappings)
>>> +    return -ENOMEM;
>>> +
>>> +    for (i = 0; i < args->num_bufs; i++) {
>>> +    copy_err = copy_from_user(, user_bufs_ptr+i, sizeof(buf));
>>
>> Whole array always should be copied at once. Please keep in mind that
>> each copy_from_user() has a cpu-time cost, there should maximum up to 2
>> copyings per job.
>>
> 
> OK. BTW, do you have some reference/numbers for this or is it based on
> grate-driver experience?

I had numbers about 2 years ago while was profiling job submission
latency using host1x-tests and for a simple jobs there was a visible
difference caused by each copy_from_user(), kmalloc() and having
firewall functions uninlined.

Of course it wasn't critical, but it's also not difficult to optimize
such things.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC PATCH v2 17/17] WIP: drm/tegra: Implement new UAPI (submit_handle_syncpts)

2020-09-11 Thread Dmitry Osipenko
09.09.2020 11:26, Mikko Perttunen пишет:
> On 9/9/20 4:24 AM, Dmitry Osipenko wrote:
>> 09.09.2020 04:13, Dmitry Osipenko пишет:
>> ...
>>> How many sync points would use an average job? Maybe it should be better
>>> to have the predefined array of sync points within the struct
>>> drm_tegra_channel_submit?
>>>
>>
>> The same question regarding the commands.
>>
>> Wouldn't it be a good idea to make both usrptr arrays of sync points and
>> commands optional by having a small fixed-size buffers within
>> drm_tegra_channel_submit? Then a majority of jobs would only need to
>> copy the gather data from userspace.
>>
> 
> Sure, I'll look into it. For syncpoints, it would be usually 1 but
> sometimes 2, so maybe make it 2. For commands, at least for downstream
> it would typically be 2 (one wait and one gather). Any opinion from
> grate-driver's point of view? Not sure if there is any recommendation
> regarding the max size of IOCTL data.

The Opentegra will need more than 2 commands. We'll need to take a look
at what are the min/max/average numbers of commands are used by
Opentegra since it combines multiple jobs into one and each job may have
several wait commands.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC PATCH v2 00/17] Host1x/TegraDRM UAPI

2020-09-11 Thread Dmitry Osipenko
09.09.2020 11:40, Mikko Perttunen пишет:
> On 9/9/20 2:36 AM, Dmitry Osipenko wrote:
>> 05.09.2020 13:34, Mikko Perttunen пишет:
>>> Hi all,
>>>
>>> here's a second revision of the Host1x/TegraDRM UAPI proposal,
>>> hopefully with most issues from v1 resolved, and also with
>>> an implementation. There are still open issues with the
>>> implementation:
>> Could you please clarify the current status of the DMA heaps. Are we
>> still going to use DMA heaps?
>>
> 
> Sorry, should have mentioned the status in the cover letter. I sent an
> email to dri-devel about how DMA heaps should be used -- I believe the
> conclusion was that it's not entirely clear, but dma-bufs should only be
> used for buffers shared between engines. So for the time being, we
> should still implement GEM for intra-TegraDRM buffers. There seems to be
> some planning ongoing to see if the different subsystem allocators can
> be unified (see dma-buf heaps talk from linux plumbers conference), but
> for now we should go for GEM.

Thanks!
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC PATCH v2 00/17] Host1x/TegraDRM UAPI

2020-09-11 Thread Dmitry Osipenko
09.09.2020 11:44, Mikko Perttunen пишет:
...
>> Could you please enumerate all the currently opened questions?
>>
> 
> Which open questions do you refer to?

Anything related to the UAPI definition that needs more thought. If
there is nothing outstanding, then good!

> The open items of v1 should be
> closed now; for fences we setup an SW timeout to prevent them from
> sticking around forever, and regarding GEM the GEM IOCTLs are again
> being used.
> 

We'll see how it will be in practice! For now it's a bit difficult to
decide what is good and what needs more improvement.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v5 27/36] memory: tegra-mc: Register as interconnect provider

2020-09-10 Thread Dmitry Osipenko
09.09.2020 11:31, Georgi Djakov пишет:
> On 8/14/20 03:06, Dmitry Osipenko wrote:
>> Now memory controller is a memory interconnection provider. This allows us
>> to use interconnect API in order to change memory configuration.
>>
>> Signed-off-by: Dmitry Osipenko 
> 
> Thanks Dmitry! Looks good to me.
> 
> Acked-by: Georgi Djakov 
> 

Thanks you!
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC PATCH v2 00/17] Host1x/TegraDRM UAPI

2020-09-09 Thread Dmitry Osipenko
05.09.2020 13:34, Mikko Perttunen пишет:
> Hi all,
> 
> here's a second revision of the Host1x/TegraDRM UAPI proposal,
> hopefully with most issues from v1 resolved, and also with
> an implementation. There are still open issues with the
> implementation:
> 
> * Relocs are now handled on TegraDRM side instead of Host1x,
>   so the firewall is not aware of them, causing submission
>   failure where the firewall is enabled. Proposed solution
>   is to move the firewall to TegraDRM side, but this hasn't
>   been done yet.
> * For the new UAPI, syncpoint recovery on job timeout is
>   disabled. What this means is that upon job timeout,
>   all further jobs using that syncpoint are cancelled,
>   and the syncpoint is marked unusable until it is freed.
>   However, there is currently a race between the timeout
>   handler and job submission, where submission can observe
>   the syncpoint in non-locked state and yet the job
>   cancellations won't cancel the new job.
> * Waiting for DMA reservation fences is not implemented yet.
> * I have only tested on Tegra186.
> 
> The series consists of three parts:
> 
> * The first part contains some fixes and improvements to
>   the Host1x driver of more general nature,
> * The second part adds the Host1x side UAPI, as well as
>   Host1x-side changes needed for the new TegraDRM UAPI,
> * The third part adds the new TegraDRM UAPI.
> 
> I have written some tests to test the new interface,
> see https://github.com/cyndis/uapi-test. Porting of proper
> userspace (e.g. opentegra, vdpau-tegra) will come once
> there is some degree of conclusion on the UAPI definition.

Could you please enumerate all the currently opened questions?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC PATCH v2 15/17] drm/tegra: Add power_on/power_off engine callbacks

2020-09-09 Thread Dmitry Osipenko
05.09.2020 13:34, Mikko Perttunen пишет:
...
> +static int vic_power_on(struct tegra_drm_client *client)
> +{
> + struct vic *vic = to_vic(client);
> +
> + return pm_runtime_get_sync(vic->dev);

Please keep in mind that RPM needs to be put in a case of error.

Maybe it would be better if driver-core could take care of
resuming/suspending client's RPM instead of putting that burden on each
client individually?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC PATCH v2 17/17] WIP: drm/tegra: Implement new UAPI (submit_handle_syncpts)

2020-09-09 Thread Dmitry Osipenko
05.09.2020 13:34, Mikko Perttunen пишет:
> +int tegra_drm_ioctl_channel_submit(struct drm_device *drm, void *data,
> +struct drm_file *file)
> +{
> + struct tegra_drm_file *fpriv = file->driver_priv;
> + struct drm_tegra_channel_submit *args = data;
> + struct drm_tegra_submit_syncpt_incr incr;
> + struct tegra_drm_job_data *job_data;
> + struct ww_acquire_ctx acquire_ctx;
> + struct tegra_drm_channel_ctx *ctx;
> + struct host1x_job *job;
> + struct gather_bo *bo;
> + u32 i;
> + int err;
> +
> + if (args->reserved[0] || args->reserved[1] || args->reserved[2] ||
> + args->reserved[3])
> + return -EINVAL;
> +
> + ctx = tegra_drm_channel_ctx_lock(fpriv, args->channel_ctx);
> + if (!ctx)
> + return -EINVAL;
> +
> + err = submit_copy_gather_data(drm, , args);
> + if (err)
> + goto unlock;
> +
> + job_data = kzalloc(sizeof(*job_data), GFP_KERNEL);
> + if (!job_data) {
> + err = -ENOMEM;
> + goto put_bo;
> + }
> +
> + err = submit_process_bufs(drm, bo, job_data, ctx, args, _ctx);
> + if (err)
> + goto free_job_data;
> +
> + err = submit_create_job(drm, , bo, ctx, args, file);
> + if (err)
> + goto free_job_data;
> +
> + err = submit_handle_syncpts(drm, job, , args);
> + if (err)
> + goto put_job;

How many sync points would use an average job? Maybe it should be better
to have the predefined array of sync points within the struct
drm_tegra_channel_submit?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: DRM_TEGRA_SUBMIT_BUF_WRITE_RELOC

2020-09-09 Thread Dmitry Osipenko
05.09.2020 13:34, Mikko Perttunen пишет:
...
> +/* Submission */
> +
> +/** Patch address of the specified mapping in the submitted gather. */
> +#define DRM_TEGRA_SUBMIT_BUF_WRITE_RELOC (1<<0)

Shouldn't the kernel driver be aware about what relocations need to be
patched? Could you please explain the purpose of this flag?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC PATCH v2 17/17] WIP: drm/tegra: Implement new UAPI

2020-09-09 Thread Dmitry Osipenko
05.09.2020 13:34, Mikko Perttunen пишет:
> +static int submit_process_bufs(struct drm_device *drm, struct gather_bo *bo,
> +struct tegra_drm_job_data *job_data,
> +struct tegra_drm_channel_ctx *ctx,
> +struct drm_tegra_channel_submit *args,
> +struct ww_acquire_ctx *acquire_ctx)
> +{
> + struct drm_tegra_submit_buf __user *user_bufs_ptr =
> + u64_to_user_ptr(args->bufs_ptr);

If assignment makes line too long, then factor it out.

  struct drm_tegra_submit_buf __user *user_bufs_ptr;

  user_bufs_ptr = u64_to_user_ptr(args->bufs_ptr);

> + struct tegra_drm_mapping *mapping;
> + struct drm_tegra_submit_buf buf;
> + unsigned long copy_err;
> + int err;
> + u32 i;
> +
> + job_data->used_mappings =
> + kcalloc(args->num_bufs, sizeof(*job_data->used_mappings), 
> GFP_KERNEL);

The checkpatch should disallow this coding style. I'd write it as:

size_t size;

size = sizeof(*job_data->used_mappings);
job_data->used_mappings = kcalloc(args->num_bufs, size..);

> + if (!job_data->used_mappings)
> + return -ENOMEM;
> +
> + for (i = 0; i < args->num_bufs; i++) {
> + copy_err = copy_from_user(, user_bufs_ptr+i, sizeof(buf));

Whole array always should be copied at once. Please keep in mind that
each copy_from_user() has a cpu-time cost, there should maximum up to 2
copyings per job.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC PATCH v2 17/17] WIP: drm/tegra: Implement new UAPI

2020-09-09 Thread Dmitry Osipenko
05.09.2020 13:34, Mikko Perttunen пишет:
> +int tegra_drm_ioctl_channel_open(struct drm_device *drm, void *data,
> +  struct drm_file *file)
> +{
> + struct tegra_drm_file *fpriv = file->driver_priv;
> + struct tegra_drm *tegra = drm->dev_private;
> + struct drm_tegra_channel_open *args = data;
> + struct tegra_drm_client *client = NULL;
> + struct tegra_drm_channel_ctx *ctx;
> + int err;
> +
> + if (args->flags || args->reserved[0] || args->reserved[1])
> + return -EINVAL;
> +
> + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> + if (!ctx)
> + return -ENOMEM;
> +
> + err = -ENODEV;
> + list_for_each_entry(client, >clients, list) {
> + if (client->base.class == args->host1x_class) {
> + err = 0;
> + break;
> + }
> + }
> + if (err)
> + goto free_ctx;
> +
> + if (client->shared_channel) {
> + ctx->channel = host1x_channel_get(client->shared_channel);
> + } else {
> + ctx->channel = host1x_channel_request(>base);
> + if (!ctx->channel) {
> + err = -EBUSY;
> + goto free_ctx;
> + }
> + }
> +
> + err = xa_alloc(>contexts, >channel_ctx, ctx,
> +XA_LIMIT(1, U32_MAX), GFP_KERNEL);
> + if (err < 0) {
> + mutex_unlock(>lock);

Looks like the lock was never taken.

> + goto put_channel;
> + }
> +
> + ctx->client = client;
> + xa_init_flags(>mappings, XA_FLAGS_ALLOC);

Why not XA_FLAGS_ALLOC1?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC PATCH v2 17/17] WIP: drm/tegra: Implement new UAPI (submit_handle_syncpts)

2020-09-09 Thread Dmitry Osipenko
09.09.2020 04:13, Dmitry Osipenko пишет:
...
> How many sync points would use an average job? Maybe it should be better
> to have the predefined array of sync points within the struct
> drm_tegra_channel_submit?
> 

The same question regarding the commands.

Wouldn't it be a good idea to make both usrptr arrays of sync points and
commands optional by having a small fixed-size buffers within
drm_tegra_channel_submit? Then a majority of jobs would only need to
copy the gather data from userspace.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC PATCH v2 00/17] Host1x/TegraDRM UAPI

2020-09-09 Thread Dmitry Osipenko
05.09.2020 13:34, Mikko Perttunen пишет:
> Hi all,
> 
> here's a second revision of the Host1x/TegraDRM UAPI proposal,
> hopefully with most issues from v1 resolved, and also with
> an implementation. There are still open issues with the
> implementation:
Could you please clarify the current status of the DMA heaps. Are we
still going to use DMA heaps?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC PATCH v2 17/17] WIP: drm/tegra: Implement new UAPI

2020-09-09 Thread Dmitry Osipenko
09.09.2020 05:10, Dmitry Osipenko пишет:
> 05.09.2020 13:34, Mikko Perttunen пишет:
>> +job->timeout = min(args->timeout_us / 1000, 1U);
>> +if (job->timeout == 0)
>> +job->timeout = 1;
> 
> clamp()
> 

Does it make sense to have timeout in microseconds?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC PATCH v2 06/17] gpu: host1x: Cleanup and refcounting for syncpoints

2020-09-09 Thread Dmitry Osipenko
05.09.2020 17:53, Mikko Perttunen пишет:
...
>> Hello, Mikko!
>>
>> What do you think about to open-code all the host1x structs by moving
>> them all out into the public linux/host1x.h? Then we could inline all
>> these trivial single-line functions by having them defined in the public
>> header. This will avoid all the unnecessary overhead by allowing
>> compiler to optimize the code nicely.
>>
>> Of course this could be a separate change and it could be done sometime
>> later, I just wanted to share this quick thought for the start of the
>> review.
>>
> 
> Hi :)
> 
> I think for such micro-optimizations we should have a benchmark to
> evaluate against. I'm not sure we have all that many function calls into
> here overall that it would make a noticeable difference. In any case, as
> you said, I'd prefer to keep further refactoring to a separate series to
> avoid growing this series too much.

The performance difference doesn't bother me, it should be insignificant
in this particular case. The amount of the exported functions is what
makes me feel uncomfortable, and especially that most of those functions
are trivial.

My concern is that doing cleanups of the upstream drivers usually not
easy. Hence it could be a good thing to put effort into restructuring
the current code before new code is added. But at first we need to have
a full-featured draft implementation that will show what parts of the
driver require refactoring.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC PATCH v2 17/17] WIP: drm/tegra: Implement new UAPI

2020-09-09 Thread Dmitry Osipenko
05.09.2020 13:34, Mikko Perttunen пишет:
> + job->timeout = min(args->timeout_us / 1000, 1U);
> + if (job->timeout == 0)
> + job->timeout = 1;

clamp()
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC PATCH v2 06/17] gpu: host1x: Cleanup and refcounting for syncpoints

2020-09-07 Thread Dmitry Osipenko
05.09.2020 13:34, Mikko Perttunen пишет:
...
> +
> +/**
> + * host1x_syncpt_put() - free a requested syncpoint
> + * @sp: host1x syncpoint
> + *
> + * Release a syncpoint previously allocated using host1x_syncpt_request(). A
> + * host1x client driver should call this when the syncpoint is no longer in
> + * use.
> + */
> +void host1x_syncpt_put(struct host1x_syncpt *sp)
> +{
> + if (!sp)
> + return;
> +
> + kref_put(>ref, syncpt_release);
> +}
> +EXPORT_SYMBOL(host1x_syncpt_put);
>  
>  void host1x_syncpt_deinit(struct host1x *host)
>  {
> @@ -471,16 +478,48 @@ unsigned int host1x_syncpt_nb_mlocks(struct host1x 
> *host)
>  }
>  
>  /**
> - * host1x_syncpt_get() - obtain a syncpoint by ID
> + * host1x_syncpt_get_by_id() - obtain a syncpoint by ID
> + * @host: host1x controller
> + * @id: syncpoint ID
> + */
> +struct host1x_syncpt *host1x_syncpt_get_by_id(struct host1x *host,
> +   unsigned int id)
> +{
> + if (id >= host->info->nb_pts)
> + return NULL;
> +
> + if (kref_get_unless_zero(>syncpt[id].ref))
> + return >syncpt[id];
> + else
> + return NULL;
> +}
> +EXPORT_SYMBOL(host1x_syncpt_get_by_id);
> +
> +/**
> + * host1x_syncpt_get_by_id_noref() - obtain a syncpoint by ID but don't
> + *   increase the refcount.
>   * @host: host1x controller
>   * @id: syncpoint ID
>   */
> -struct host1x_syncpt *host1x_syncpt_get(struct host1x *host, unsigned int id)
> +struct host1x_syncpt *host1x_syncpt_get_by_id_noref(struct host1x *host,
> + unsigned int id)
>  {
>   if (id >= host->info->nb_pts)
>   return NULL;
>  
> - return host->syncpt + id;
> + return >syncpt[id];
> +}
> +EXPORT_SYMBOL(host1x_syncpt_get_by_id_noref);
> +
> +/**
> + * host1x_syncpt_get() - increment syncpoint refcount
> + * @sp: syncpoint
> + */
> +struct host1x_syncpt *host1x_syncpt_get(struct host1x_syncpt *sp)
> +{
> + kref_get(>ref);
> +
> + return sp;
>  }
>  EXPORT_SYMBOL(host1x_syncpt_get);

Hello, Mikko!

What do you think about to open-code all the host1x structs by moving
them all out into the public linux/host1x.h? Then we could inline all
these trivial single-line functions by having them defined in the public
header. This will avoid all the unnecessary overhead by allowing
compiler to optimize the code nicely.

Of course this could be a separate change and it could be done sometime
later, I just wanted to share this quick thought for the start of the
review.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 16/18] staging/media/tegra-vde: Clean up IOMMU workaround

2020-08-31 Thread Dmitry Osipenko
27.08.2020 18:54, Thierry Reding пишет:
...
>> The Tegra DRM has a very special quirk for ARM32 that was added in this
>> commit [2] and driver relies on checking of whether explicit or implicit
>> IOMMU is used in order to activate the quirk.
>>
>> [2]
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=273da5a046965ccf0ec79eb63f2d5173467e20fa
>>
>> Once the implicit IOMMU is used for the DRM driver, the quirk no longer
>> works (if I'm not missing something). This problem needs to be resolved
>> before implicit IOMMU could be used by the Tegra DRM on ARM32.
...
> I do have a patch lying around somewhere that implements the mapping
> cache that was referenced in the above commit. Let me know if I should
> dig that up and send it out.

Hello, Thierry! It certainly will be interesting to take a look at yours
patch!

I think that the caching shouldn't be strictly necessary for keeping the
current workaround working and it should be possible to keep the code
as-is by replacing the domain-type checking with the SoC-generation
check in the Tegra DRM driver.

In general, IMO it should be better to stash the complex changes until
we'll get closer to adopting the new UAPI as it will certainly touch the
aspect of the per-device mappings. But if yours patch is less than 100
LOC, then maybe we could consider applying it right now!
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v5 13/36] PM / devfreq: tegra30: Use MC timings for building OPP table

2020-08-29 Thread Dmitry Osipenko
28.08.2020 04:47, Chanwoo Choi пишет:
> Hi,
...
>> Hence the NULL-checking is unnecessary.
>>
>> When I first encountered the of_device_get_match_data(), I was also
>> thinking that adding the NULL-checks is a good idea, but later on
>> somebody pointed out to me (maybe Thierry) that it's unnecessary to do.
> 
> OK. Thanks.
> 
>>
 +
 +  mc = tegra_get_memory_controller(soc_data->mc_compatible);
 +  if (IS_ERR(mc))
 +  return PTR_ERR(mc);
>>>
>>> You better to add error log.
>>
>> In practice we should get only -EPROBE_DEFER here ever. I'll consider
>> adding the message in the next revision, at least just for consistency.
> 
> In order to handle -EPROBE_DEFER, recommend the using of dev_err_probe().

Hello, Chanwoo!

Thank you for the suggestion! I wasn't aware about the dev_err_probe()
until recently and will use this new helper in the v6!

Thanks!
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 16/18] staging/media/tegra-vde: Clean up IOMMU workaround

2020-08-28 Thread Dmitry Osipenko
24.08.2020 17:01, Robin Murphy пишет:
...
>> Robin, thank you very much for the clarifications!
>>
>> In accordance to yours comments, this patch can't be applied until Tegra
>> SMMU will support IOMMU_DOMAIN_IDENTITY and implement def_domain_type()
>> callback that returns IOMMU_DOMAIN_IDENTITY for the VDE device.
>>
>> Otherwise you're breaking the VDE driver because
>> dma_buf_map_attachment() [1] returns the IOMMU SGT of the implicit
>> domain which is then mapped into the VDE's explicit domain [2], and this
>> is a nonsense.
> 
> It's true that iommu_dma_ops will do some work in the unattached default
> domain, but non-coherent cache maintenance will still be performed
> correctly on the underlying memory, which is really all that you care
> about for this case. As for tegra_vde_iommu_map(), that seems to do the
> right thing in only referencing the physical side of the scatterlist
> (via iommu_map_sg()) and ignoring the DMA side, so things ought to work
> out OK even if it is a little non-obvious.

I'll need to double-check this, it's indeed not clear to me right now.

I see that if Tegra DRM driver uses implicit IOMMU domain, then when VDE
driver imports DMA-buf from Terga DRM and the imported buffer will be
auto-mapped to the implicit VDE IOVA [1].

[1]
https://elixir.bootlin.com/linux/v5.9-rc2/source/drivers/gpu/drm/tegra/gem.c#L574

>> Hence, either VDE driver should bypass iommu_dma_ops from the start or
>> it needs a way to kick out the ops, like it does this using ARM's
>> arm_iommu_detach_device().
>>
>>
>> The same applies to the Tegra GPU devices, otherwise you're breaking
>> them as well because Tegra DRM is sensible to implicit vs explicit
>> domain.
> 
> Note that Tegra DRM will only be as broken as its current state on
> arm64, and I was under the impression that that was OK now - at least I
> don't recall seeing any complaints since 43c5bf11a610. Although that
> commit and the one before it are resolving the scalability issue that
> they describe, it was very much in my mind at the time that they also
> have the happy side-effect described above - the default domain isn't
> *completely* out of the way, but it's far enough that sensible cases
> should be able to work as expected.

The Tegra DRM has a very special quirk for ARM32 that was added in this
commit [2] and driver relies on checking of whether explicit or implicit
IOMMU is used in order to activate the quirk.

[2]
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=273da5a046965ccf0ec79eb63f2d5173467e20fa

Once the implicit IOMMU is used for the DRM driver, the quirk no longer
works (if I'm not missing something). This problem needs to be resolved
before implicit IOMMU could be used by the Tegra DRM on ARM32.

>> BTW, I tried to apply this series and T30 doesn't boot anymore. I don't
>> have more info for now.
> 
> Yeah, I'm still trying to get to the bottom of whether it's actually
> working as intended at all, even on my RK3288. So far my debugging
> instrumentation has been confusingly inconclusive :/

Surely it will take some time to resolve all the problems and it's great
that you're pushing this work!

I'll try to help with fixing the ARM32 Tegra side of the problems. I
added this to my "TODO" list and should be able to take a closer look
during of this/next weeks!
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 12/18] iommu/tegra-gart: Add IOMMU_DOMAIN_DMA support

2020-08-24 Thread Dmitry Osipenko
21.08.2020 03:28, Robin Murphy пишет:
...
>> Will a returned NULL tell to IOMMU core that implicit domain shouldn't
>> be used? Is it possible to leave this driver as-is?
> 
> The aim of this patch was just to make the conversion without functional
> changes wherever possible, i.e. maintain an equivalent to the existing
> ARM behaviour of allocating its own implicit domains for everything. It
> doesn't represent any judgement of whether that was ever appropriate for
> this driver in the first place ;)
> 
> Hopefully my other reply already covered the degree of control drivers
> can have with proper default domains, but do shout if anything wasn't
> clear.

Thank you for the detailed comments! I wasn't watching closely all the
recent iommu/ changes and yours clarification is very helpful!

My current understanding is that the GART driver will need to support
the IOMMU_DOMAIN_IDENTITY and set def_domain_type to
IOMMU_DOMAIN_IDENTITY for all devices.

Meanwhile, today's upstream drivers don't use GART, hence this patch
should be okay. Although, it's a bit unlikely that the IOMMU_DOMAIN_DMA
type will ever be useful for the GART, and thus, I'm still thinking that
will be a bit nicer to keep GART driver as-is for now.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 16/18] staging/media/tegra-vde: Clean up IOMMU workaround

2020-08-24 Thread Dmitry Osipenko
21.08.2020 03:11, Robin Murphy пишет:
...
>> Hello, Robin! Thank you for yours work!
>>
>> Some drivers, like this Tegra VDE (Video Decoder Engine) driver for
>> example, do not want to use implicit IOMMU domain.
> 
> That isn't (intentionally) changing here - the only difference should be
> that instead of having the ARM-special implicit domain, which you have
> to kick out of the way with the ARM-specific API before you're able to
> attach your own domain, the implicit domain is now a proper IOMMU API
> default domain, which automatically gets bumped by your attach. The
> default domains should still only be created in the same cases that the
> ARM dma_iommu_mappings were.
> 
>> Tegra VDE driver
>> relies on explicit IOMMU domain in a case of Tegra SMMU because VDE
>> hardware can't access last page of the AS and because driver wants to
>> reserve some fixed addresses [1].
>>
>> [1]
>> https://elixir.bootlin.com/linux/v5.9-rc1/source/drivers/staging/media/tegra-vde/iommu.c#L100
>>
>>
>> Tegra30 SoC supports up to 4 domains, hence it's not possible to afford
>> wasting unused implicit domains. I think this needs to be addressed
>> before this patch could be applied.
> 
> Yeah, there is one subtle change in behaviour from removing the ARM
> layer on top of the core API, in that the IOMMU driver will no longer
> see an explicit detach call. Thus it does stand to benefit from being a
> bit cleverer about noticing devices being moved from one domain to
> another by an attach call, either by releasing the hardware context for
> the inactive domain once the device(s) are moved across to the new one,
> or by simply reprogramming the hardware context in-place for the new
> domain's address space without allocating a new one at all (most of the
> drivers that don't have multiple contexts already handle the latter
> approach quite well).
> 
>> Would it be possible for IOMMU drivers to gain support for filtering out
>> devices in iommu_domain_alloc(dev, type)? Then perhaps Tegra SMMU driver
>> could simply return NULL in a case of type=IOMMU_DOMAIN_DMA and
>> dev=tegra-vde.
> 
> If you can implement IOMMU_DOMAIN_IDENTITY by allowing the relevant
> devices to bypass translation entirely without needing a hardware
> context (or at worst, can spare one context which all identity-mapped
> logical domains can share), then you could certainly do that kind of
> filtering with the .def_domain_type callback if you really wanted to. As
> above, the intent is that that shouldn't be necessary for this
> particular case, since only one of a group's default domain and
> explicitly attached domain can be live at any given time, so the driver
> should be able to take advantage of that.
> 
> If you simply have more active devices (groups) than available contexts
> then yes, you probably would want to do some filtering to decide who
> deserves a translation domain and who doesn't, but in that case you
> should already have had a long-standing problem with the ARM implicit
> domains.
> 
>> Alternatively, the Tegra SMMU could be changed such that the devices
>> will be attached to a domain at the time of a first IOMMU mapping
>> invocation instead of attaching at the time of attach_dev() callback
>> invocation.
>>
>> Or maybe even IOMMU core could be changed to attach devices at the time
>> of the first IOMMU mapping invocation? This could be a universal
>> solution for all drivers.
> 
> I suppose technically you could do that within an IOMMU driver already
> (similar to how some defer most of setup that logically belongs to
> ->domain_alloc until the first ->attach_dev). It's a bit grim from the
> caller's PoV though, in terms of the failure mode being non-obvious and
> having no real way to recover. Again, you'd be better off simply making
> decisions up-front at domain_alloc or attach time based on the domain type.

Robin, thank you very much for the clarifications!

In accordance to yours comments, this patch can't be applied until Tegra
SMMU will support IOMMU_DOMAIN_IDENTITY and implement def_domain_type()
callback that returns IOMMU_DOMAIN_IDENTITY for the VDE device.

Otherwise you're breaking the VDE driver because
dma_buf_map_attachment() [1] returns the IOMMU SGT of the implicit
domain which is then mapped into the VDE's explicit domain [2], and this
is a nonsense.

[1]
https://elixir.bootlin.com/linux/v5.9-rc1/source/drivers/staging/media/tegra-vde/dmabuf-cache.c#L102

[2]
https://elixir.bootlin.com/linux/v5.9-rc1/source/drivers/staging/media/tegra-vde/dmabuf-cache.c#L122

Hence, either VDE driver should bypass iommu_dma_ops from the start or
it needs a way to kick out the ops, like it does this using ARM's
arm_iommu_detach_device().


The same applies to the Tegra GPU devices, otherwise you're breaking
them as well because Tegra DRM is sensible to implicit vs explicit domain.


BTW, I tried to apply this series and T30 doesn't boot anymore. I don't
have more info for now.
___

Re: [PATCH 16/18] staging/media/tegra-vde: Clean up IOMMU workaround

2020-08-21 Thread Dmitry Osipenko
20.08.2020 18:08, Robin Murphy пишет:
> Now that arch/arm is wired up for default domains and iommu-dma, we no
> longer need to work around the arch-private mapping.
> 
> Signed-off-by: Robin Murphy 
> ---
>  drivers/staging/media/tegra-vde/iommu.c | 12 
>  1 file changed, 12 deletions(-)
> 
> diff --git a/drivers/staging/media/tegra-vde/iommu.c 
> b/drivers/staging/media/tegra-vde/iommu.c
> index 6af863d92123..4f770189ed34 100644
> --- a/drivers/staging/media/tegra-vde/iommu.c
> +++ b/drivers/staging/media/tegra-vde/iommu.c
> @@ -10,10 +10,6 @@
>  #include 
>  #include 
>  
> -#if IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)
> -#include 
> -#endif
> -
>  #include "vde.h"
>  
>  int tegra_vde_iommu_map(struct tegra_vde *vde,
> @@ -70,14 +66,6 @@ int tegra_vde_iommu_init(struct tegra_vde *vde)
>   if (!vde->group)
>   return 0;
>  
> -#if IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)
> - if (dev->archdata.mapping) {
> - struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev);
> -
> - arm_iommu_detach_device(dev);
> - arm_iommu_release_mapping(mapping);
> - }
> -#endif
>   vde->domain = iommu_domain_alloc(_bus_type);
>   if (!vde->domain) {
>   err = -ENOMEM;
> 

Hello, Robin! Thank you for yours work!

Some drivers, like this Tegra VDE (Video Decoder Engine) driver for
example, do not want to use implicit IOMMU domain. Tegra VDE driver
relies on explicit IOMMU domain in a case of Tegra SMMU because VDE
hardware can't access last page of the AS and because driver wants to
reserve some fixed addresses [1].

[1]
https://elixir.bootlin.com/linux/v5.9-rc1/source/drivers/staging/media/tegra-vde/iommu.c#L100

Tegra30 SoC supports up to 4 domains, hence it's not possible to afford
wasting unused implicit domains. I think this needs to be addressed
before this patch could be applied.

Would it be possible for IOMMU drivers to gain support for filtering out
devices in iommu_domain_alloc(dev, type)? Then perhaps Tegra SMMU driver
could simply return NULL in a case of type=IOMMU_DOMAIN_DMA and
dev=tegra-vde.

Alternatively, the Tegra SMMU could be changed such that the devices
will be attached to a domain at the time of a first IOMMU mapping
invocation instead of attaching at the time of attach_dev() callback
invocation.

Or maybe even IOMMU core could be changed to attach devices at the time
of the first IOMMU mapping invocation? This could be a universal
solution for all drivers.

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 12/18] iommu/tegra-gart: Add IOMMU_DOMAIN_DMA support

2020-08-21 Thread Dmitry Osipenko
20.08.2020 18:08, Robin Murphy пишет:
> Now that arch/arm is wired up for default domains and iommu-dma,
> implement the corresponding driver-side support for DMA domains.
> 
> Signed-off-by: Robin Murphy 
> ---
>  drivers/iommu/tegra-gart.c | 17 -
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iommu/tegra-gart.c b/drivers/iommu/tegra-gart.c
> index fac720273889..e081387080f6 100644
> --- a/drivers/iommu/tegra-gart.c
> +++ b/drivers/iommu/tegra-gart.c
> @@ -9,6 +9,7 @@
>  
>  #define dev_fmt(fmt) "gart: " fmt
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -145,16 +146,22 @@ static struct iommu_domain 
> *gart_iommu_domain_alloc(unsigned type)
>  {
>   struct iommu_domain *domain;

Hello, Robin!

Tegra20 GART isn't a real IOMMU, but a small relocation aperture. We
would only want to use it for a temporal mappings (managed by GPU
driver) for the time while GPU hardware is busy and working with a
sparse DMA buffers, the driver will take care of unmapping the sparse
buffers once GPU work is finished [1]. In a case of contiguous DMA
buffers, we want to bypass the IOMMU and use buffer's phys address
because GART aperture is small and all buffers simply can't fit into
GART for a complex GPU operations that involve multiple buffers [2][3].
The upstream GPU driver still doesn't support GART, but eventually it
needs to be changed.

[1]
https://github.com/grate-driver/linux/blob/master/drivers/gpu/drm/tegra/gart.c#L489

[2]
https://github.com/grate-driver/linux/blob/master/drivers/gpu/drm/tegra/gart.c#L542

[3]
https://github.com/grate-driver/linux/blob/master/drivers/gpu/drm/tegra/uapi/patching.c#L90

> - if (type != IOMMU_DOMAIN_UNMANAGED)
> + if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
>   return NULL;

Will a returned NULL tell to IOMMU core that implicit domain shouldn't
be used? Is it possible to leave this driver as-is?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 16/18] staging/media/tegra-vde: Clean up IOMMU workaround

2020-08-21 Thread Dmitry Osipenko
20.08.2020 22:51, Dmitry Osipenko пишет:
> Alternatively, the Tegra SMMU could be changed such that the devices
> will be attached to a domain at the time of a first IOMMU mapping
> invocation instead of attaching at the time of attach_dev() callback
> invocation.
> 
> Or maybe even IOMMU core could be changed to attach devices at the time
> of the first IOMMU mapping invocation? This could be a universal
> solution for all drivers.

Although, please scratch this :) I'll need to revisit how DMA mapping
API works.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH RESEND v12 0/4] Panel rotation patches

2020-08-17 Thread Dmitry Osipenko
16.08.2020 18:17, Sam Ravnborg пишет:
> Hi Dmitry
> 
> On Fri, Aug 14, 2020 at 12:56:05AM +0300, Dmitry Osipenko wrote:
>> Hello!
>>
>> This series adds support for display panel's DT rotation property. It's a
>> continuation of the work that was initially started by Derek Basehore for
>> the panel driver that is used by some Mediatek device [1]. I picked up the
>> Derek's patches and added my t-b and r-b tags to them, I also added
>> rotation support to the panel-lvds and panel-simple drivers.
>>
>> We need the rotation support for the Nexus 7 tablet device which is now
>> supported by the upstream kernel, the device has display panel mounted
>> upside-down and it uses panel-lvds [2].
>>
>> [1] https://lkml.org/lkml/2020/3/5/1119
>> [2] 
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm/boot/dts/tegra30-asus-nexus7-grouper-common.dtsi?#n1036
>>
>> Changelog:
>>
>> v12: - No code changes. The v11 missed v5.9 release, re-sending patches
>>for the v5.10 kernel. Please review and apply patches to linux-next,
>>thanks in advance!
>>
>> v11: - This series is factored out from this patchset [3] because these
>>patches do not have hard dependency on the Tegra DRM patches and
>>it should be nicer to review and apply the properly grouped patches.
>>
>>  - Initially [3] only touched the panel-lvds driver and Emil Velikov
>>suggested that it will be better to support more panels in the review
>>comments to [3]. So I included the Derek's patch for the BOE panel
>>and added rotation support to the panel-simple driver. I tested that
>>panel-lvds and panel-simple work properly with the rotated panel using
>>the Opentegra Xorg driver [4] and Wayland Weston [5].
>>
>>  - The panel-lvds driver now prints a error message if rotation property
>>fails to be parsed.
>>
>> [3] https://lore.kernel.org/lkml/20200614200121.14147-1-dig...@gmail.com/
>> [4] 
>> https://github.com/grate-driver/xf86-video-opentegra/commit/28eb20a3959bbe5bc3a3b67e55977093fd5114ca
>> [5] https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/315
>>
>> Derek Basehore (2):
>>   drm/panel: Add helper for reading DT rotation
>>   drm/panel: Read panel orientation for BOE TV101WUM-NL6
>>
>> Dmitry Osipenko (2):
>>   drm/panel: lvds: Read panel orientation
>>   drm/panel-simple: Read panel orientation
> 
> Thanks for your persistence with these patches.
> While applying I made a few updates:
> - fixed two trivial checkpatch warnings
> - small update to kernel-doc for the new function, to better match
>   surrounding wording
> - added error message to panel-boe-tv101wum-nl6.c when failed to get
>   orientation
> - use same wording in all error messages and use "orientation" and not
>   rotation as this matches the called function

Hello, Sam! Very nice to see the progress! Thank you very much!
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 22/36] dt-bindings: host1x: Document new interconnect properties

2020-08-17 Thread Dmitry Osipenko
Most of Host1x devices have at least one memory client. These clients
are directly connected to the memory controller. The new interconnect
properties represent the memory client's connection to the memory
controller.

Signed-off-by: Dmitry Osipenko 
---
 .../display/tegra/nvidia,tegra20-host1x.txt   | 68 +++
 1 file changed, 68 insertions(+)

diff --git 
a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt 
b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
index 47319214b5f6..dd94b8bf4ae3 100644
--- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
+++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
@@ -20,6 +20,10 @@ Required properties:
 - reset-names: Must include the following entries:
   - host1x
 
+Each host1x client module having to perform DMA through the Memory Controller
+should have the interconnect endpoints set to the Memory Client and External
+Memory respectively.
+
 The host1x top-level node defines a number of children, each representing one
 of the following host1x client modules:
 
@@ -36,6 +40,12 @@ of the following host1x client modules:
   - reset-names: Must include the following entries:
 - mpe
 
+  Optional properties:
+  - interconnects: Must contain entry for the MPE memory clients.
+  - interconnect-names: Must include name of the interconnect path for each
+interconnect entry. Consult TRM documentation for information about
+available memory clients, see MEMORY CONTROLLER section.
+
 - vi: video input
 
   Required properties:
@@ -65,6 +75,12 @@ of the following host1x client modules:
   - power-domains: Must include sor powergate node as csicil is in
 SOR partition.
 
+  Optional properties:
+  - interconnects: Must contain entry for the VI memory clients.
+  - interconnect-names: Must include name of the interconnect path for each
+interconnect entry. Consult TRM documentation for information about
+available memory clients, see MEMORY CONTROLLER section.
+
 - epp: encoder pre-processor
 
   Required properties:
@@ -78,6 +94,12 @@ of the following host1x client modules:
   - reset-names: Must include the following entries:
 - epp
 
+  Optional properties:
+  - interconnects: Must contain entry for the EPP memory clients.
+  - interconnect-names: Must include name of the interconnect path for each
+interconnect entry. Consult TRM documentation for information about
+available memory clients, see MEMORY CONTROLLER section.
+
 - isp: image signal processor
 
   Required properties:
@@ -91,6 +113,12 @@ of the following host1x client modules:
   - reset-names: Must include the following entries:
 - isp
 
+  Optional properties:
+  - interconnects: Must contain entry for the ISP memory clients.
+  - interconnect-names: Must include name of the interconnect path for each
+interconnect entry. Consult TRM documentation for information about
+available memory clients, see MEMORY CONTROLLER section.
+
 - gr2d: 2D graphics engine
 
   Required properties:
@@ -104,6 +132,12 @@ of the following host1x client modules:
   - reset-names: Must include the following entries:
 - 2d
 
+  Optional properties:
+  - interconnects: Must contain entry for the GR2D memory clients.
+  - interconnect-names: Must include name of the interconnect path for each
+interconnect entry. Consult TRM documentation for information about
+available memory clients, see MEMORY CONTROLLER section.
+
 - gr3d: 3D graphics engine
 
   Required properties:
@@ -122,6 +156,12 @@ of the following host1x client modules:
 - 3d
 - 3d2 (Only required on SoCs with two 3D clocks)
 
+  Optional properties:
+  - interconnects: Must contain entry for the GR3D memory clients.
+  - interconnect-names: Must include name of the interconnect path for each
+interconnect entry. Consult TRM documentation for information about
+available memory clients, see MEMORY CONTROLLER section.
+
 - dc: display controller
 
   Required properties:
@@ -149,6 +189,10 @@ of the following host1x client modules:
   - nvidia,hpd-gpio: specifies a GPIO used for hotplug detection
   - nvidia,edid: supplies a binary EDID blob
   - nvidia,panel: phandle of a display panel
+  - interconnects: Must contain entry for the DC memory clients.
+  - interconnect-names: Must include name of the interconnect path for each
+interconnect entry. Consult TRM documentation for information about
+available memory clients, see MEMORY CONTROLLER section.
 
 - hdmi: High Definition Multimedia Interface
 
@@ -297,6 +341,12 @@ of the following host1x client modules:
   - reset-names: Must include the following entries:
 - vic
 
+  Optional properties:
+  - interconnects: Must contain entry for the VIC memory clients.
+  - interconnect-names: Must include name of the interconnect path for each
+interconnect entry. Consult TRM documentation for information about

[PATCH v5 04/36] memory: tegra20-emc: Make driver modular

2020-08-17 Thread Dmitry Osipenko
This patch adds modularization support to the Tegra20 EMC driver. Driver
now can be compiled as a loadable kernel module.

Signed-off-by: Dmitry Osipenko 
---
 drivers/memory/tegra/Kconfig   |  2 +-
 drivers/memory/tegra/tegra20-emc.c | 17 -
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
index 9f0a96bf9ccc..7e0e1ef87763 100644
--- a/drivers/memory/tegra/Kconfig
+++ b/drivers/memory/tegra/Kconfig
@@ -8,7 +8,7 @@ config TEGRA_MC
  NVIDIA Tegra SoCs.
 
 config TEGRA20_EMC
-   bool "NVIDIA Tegra20 External Memory Controller driver"
+   tristate "NVIDIA Tegra20 External Memory Controller driver"
default y
depends on ARCH_TEGRA_2x_SOC
help
diff --git a/drivers/memory/tegra/tegra20-emc.c 
b/drivers/memory/tegra/tegra20-emc.c
index 83ca46345aba..5aa3a1da2975 100644
--- a/drivers/memory/tegra/tegra20-emc.c
+++ b/drivers/memory/tegra/tegra20-emc.c
@@ -724,6 +724,13 @@ static int tegra_emc_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, emc);
tegra_emc_debugfs_init(emc);
 
+   /*
+* Don't allow the kernel module to be unloaded. Unloading adds some
+* extra complexity which doesn't really worth the effort in a case of
+* this driver.
+*/
+   try_module_get(THIS_MODULE);
+
return 0;
 
 unset_cb:
@@ -736,6 +743,7 @@ static const struct of_device_id tegra_emc_of_match[] = {
{ .compatible = "nvidia,tegra20-emc", },
{},
 };
+MODULE_DEVICE_TABLE(of, tegra_emc_of_match);
 
 static struct platform_driver tegra_emc_driver = {
.probe = tegra_emc_probe,
@@ -745,9 +753,8 @@ static struct platform_driver tegra_emc_driver = {
.suppress_bind_attrs = true,
},
 };
+module_platform_driver(tegra_emc_driver);
 
-static int __init tegra_emc_init(void)
-{
-   return platform_driver_register(_emc_driver);
-}
-subsys_initcall(tegra_emc_init);
+MODULE_AUTHOR("Dmitry Osipenko ");
+MODULE_DESCRIPTION("NVIDIA Tegra20 EMC driver");
+MODULE_LICENSE("GPL v2");
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 35/36] drm/tegra: dc: Tune up high priority request controls for Tegra20

2020-08-17 Thread Dmitry Osipenko
Tegra20 has a high-priority-request control that allows to configure
when display's memory client should perform read requests with a higher
priority (Tegra30+ uses other means like Latency Allowance).

This patch changes the controls configuration in order to get a more
aggressive memory prefetching, which allows to reliably avoid FIFO
underflow when running on a lower memory frequency. This allow us
safely drop the memory bandwidth requirement by about two times in a
most popular use-cases (only one display active, video overlay inactive,
no scaling is done).

Signed-off-by: Dmitry Osipenko 
---
 drivers/gpu/drm/tegra/dc.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 850fbcebefc2..4b062408467e 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -1999,12 +1999,12 @@ static void tegra_crtc_atomic_enable(struct drm_crtc 
*crtc,
tegra_dc_writel(dc, value, DC_CMD_INT_POLARITY);
 
/* initialize timer */
-   value = CURSOR_THRESHOLD(0) | WINDOW_A_THRESHOLD(0x20) |
-   WINDOW_B_THRESHOLD(0x20) | WINDOW_C_THRESHOLD(0x20);
+   value = CURSOR_THRESHOLD(0) | WINDOW_A_THRESHOLD(0x70) |
+   WINDOW_B_THRESHOLD(0x30) | WINDOW_C_THRESHOLD(0x70);
tegra_dc_writel(dc, value, DC_DISP_DISP_MEM_HIGH_PRIORITY);
 
-   value = CURSOR_THRESHOLD(0) | WINDOW_A_THRESHOLD(1) |
-   WINDOW_B_THRESHOLD(1) | WINDOW_C_THRESHOLD(1);
+   value = CURSOR_THRESHOLD(0) | WINDOW_A_THRESHOLD(0) |
+   WINDOW_B_THRESHOLD(0) | WINDOW_C_THRESHOLD(0);
tegra_dc_writel(dc, value, 
DC_DISP_DISP_MEM_HIGH_PRIORITY_TIMER);
 
value = VBLANK_INT | WIN_A_UF_INT | WIN_B_UF_INT | WIN_C_UF_INT 
|
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 11/36] PM / devfreq: tegra30: Silence deferred probe error

2020-08-17 Thread Dmitry Osipenko
Tegra EMC driver was turned into a regular kernel driver, it also could
be compiled as a loadable kernel module now. Hence EMC clock isn't
guaranteed to be available and clk_get("emc") may return -EPROBE_DEFER and
there is no good reason to spam KMSG with a error about missing EMC clock
in this case, so let's silence the deferred probe error.

Acked-by: Chanwoo Choi 
Signed-off-by: Dmitry Osipenko 
---
 drivers/devfreq/tegra30-devfreq.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/devfreq/tegra30-devfreq.c 
b/drivers/devfreq/tegra30-devfreq.c
index e94a27804c20..423dd35c95b3 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -801,9 +801,12 @@ static int tegra_devfreq_probe(struct platform_device 
*pdev)
}
 
tegra->emc_clock = devm_clk_get(>dev, "emc");
-   if (IS_ERR(tegra->emc_clock)) {
-   dev_err(>dev, "Failed to get emc clock\n");
-   return PTR_ERR(tegra->emc_clock);
+   err = PTR_ERR_OR_ZERO(tegra->emc_clock);
+   if (err) {
+   if (err != -EPROBE_DEFER)
+   dev_err(>dev, "Failed to get emc clock: %d\n",
+   err);
+   return err;
}
 
err = platform_get_irq(pdev, 0);
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH RESEND v12 4/4] drm/panel-simple: Read panel orientation

2020-08-17 Thread Dmitry Osipenko
The panel orientation needs to parsed from a device-tree and assigned to
the panel's connector in order to make orientation property available to
userspace. That's what this patch does for the panel-simple driver.

Signed-off-by: Dmitry Osipenko 
---
 drivers/gpu/drm/panel/panel-simple.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index cb6550d37e85..6e3e99a30d85 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -112,6 +112,8 @@ struct panel_simple {
struct gpio_desc *hpd_gpio;
 
struct drm_display_mode override_mode;
+
+   enum drm_panel_orientation orientation;
 };
 
 static inline struct panel_simple *to_panel_simple(struct drm_panel *panel)
@@ -371,6 +373,9 @@ static int panel_simple_get_modes(struct drm_panel *panel,
/* add hard-coded panel modes */
num += panel_simple_get_non_edid_modes(p, connector);
 
+   /* set up connector's "panel orientation" property */
+   drm_connector_set_panel_orientation(connector, p->orientation);
+
return num;
 }
 
@@ -530,6 +535,12 @@ static int panel_simple_probe(struct device *dev, const 
struct panel_desc *desc)
return err;
}
 
+   err = of_drm_get_panel_orientation(dev->of_node, >orientation);
+   if (err) {
+   dev_err(dev, "failed to parse rotation property: %d\n", err);
+   return err;
+   }
+
ddc = of_parse_phandle(dev->of_node, "ddc-i2c-bus", 0);
if (ddc) {
panel->ddc = of_find_i2c_adapter_by_node(ddc);
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 18/36] dt-bindings: memory: tegra20: mc: Document new interconnect property

2020-08-17 Thread Dmitry Osipenko
Memory controller is interconnected with memory clients and with the
external memory controller. Document new interconnect property which
turns memory controller into interconnect provider.

Acked-by: Rob Herring 
Signed-off-by: Dmitry Osipenko 
---
 .../bindings/memory-controllers/nvidia,tegra20-mc.txt  | 3 +++
 1 file changed, 3 insertions(+)

diff --git 
a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.txt 
b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.txt
index e55328237df4..739b7c6f2e26 100644
--- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.txt
+++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.txt
@@ -16,6 +16,8 @@ Required properties:
   IOMMU specifier needed to encode an address. GART supports only a single
   address space that is shared by all devices, therefore no additional
   information needed for the address encoding.
+- #interconnect-cells : Should be 1. This cell represents memory client.
+  The assignments may be found in header file 
.
 
 Example:
mc: memory-controller@7000f000 {
@@ -27,6 +29,7 @@ Example:
interrupts = ;
#reset-cells = <1>;
#iommu-cells = <0>;
+   #interconnect-cells = <1>;
};
 
video-codec@6001a000 {
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 33/36] memory: tegra30-emc: Register as interconnect provider

2020-08-17 Thread Dmitry Osipenko
Now external memory controller is a memory interconnection provider.
This allows us to use interconnect API to change memory configuration.

Signed-off-by: Dmitry Osipenko 
---
 drivers/memory/tegra/tegra30-emc.c | 110 +
 1 file changed, 110 insertions(+)

diff --git a/drivers/memory/tegra/tegra30-emc.c 
b/drivers/memory/tegra/tegra30-emc.c
index 0cd39bf5f393..29448d02f750 100644
--- a/drivers/memory/tegra/tegra30-emc.c
+++ b/drivers/memory/tegra/tegra30-emc.c
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -327,6 +328,7 @@ struct tegra_emc {
struct device *dev;
struct tegra_mc *mc;
struct notifier_block clk_nb;
+   struct icc_provider provider;
struct clk *clk;
void __iomem *regs;
unsigned int irq;
@@ -1264,6 +1266,113 @@ static void tegra_emc_debugfs_init(struct tegra_emc 
*emc)
emc, _emc_debug_max_rate_fops);
 }
 
+static inline struct tegra_emc *
+to_tegra_emc_provider(struct icc_provider *provider)
+{
+   return container_of(provider, struct tegra_emc, provider);
+}
+
+static struct icc_node *
+emc_of_icc_xlate(struct of_phandle_args *spec, void *data)
+{
+   struct icc_provider *provider = data;
+   struct icc_node *node;
+
+   /* External Memory is the only possible ICC route */
+   list_for_each_entry(node, >nodes, node_list) {
+   if (node->id == TEGRA_ICC_EMEM)
+   return node;
+   }
+
+   return ERR_PTR(-EINVAL);
+}
+
+static int emc_icc_set(struct icc_node *src, struct icc_node *dst)
+{
+   struct tegra_emc *emc = to_tegra_emc_provider(dst->provider);
+   unsigned long long rate = icc_units_to_bps(dst->avg_bw);
+   unsigned int dram_data_bus_width_bytes = 4;
+   unsigned int ddr = 2;
+   int err;
+
+   do_div(rate, ddr * dram_data_bus_width_bytes);
+   rate = min_t(u64, rate, U32_MAX);
+
+   err = clk_set_min_rate(emc->clk, rate);
+   if (err)
+   return err;
+
+   err = clk_set_rate(emc->clk, rate);
+   if (err)
+   return err;
+
+   return 0;
+}
+
+static int emc_icc_aggregate(struct icc_node *node,
+u32 tag, u32 avg_bw, u32 peak_bw,
+u32 *agg_avg, u32 *agg_peak)
+{
+   *agg_avg = min((u64)avg_bw + (*agg_avg), (u64)U32_MAX);
+   *agg_peak = max(*agg_peak, peak_bw);
+
+   return 0;
+}
+
+static int tegra_emc_interconnect_init(struct tegra_emc *emc)
+{
+   struct icc_node *node;
+   int err;
+
+   /* older device-trees don't have interconnect properties */
+   if (!of_find_property(emc->dev->of_node, "#interconnect-cells", NULL))
+   return 0;
+
+   emc->provider.dev = emc->dev;
+   emc->provider.set = emc_icc_set;
+   emc->provider.data = >provider;
+   emc->provider.xlate = emc_of_icc_xlate;
+   emc->provider.aggregate = emc_icc_aggregate;
+
+   err = icc_provider_add(>provider);
+   if (err)
+   goto err_msg;
+
+   /* create External Memory Controller node */
+   node = icc_node_create(TEGRA_ICC_EMC);
+   err = PTR_ERR_OR_ZERO(node);
+   if (err)
+   goto del_provider;
+
+   node->name = "External Memory Controller";
+   icc_node_add(node, >provider);
+
+   /* link External Memory Controller to External Memory (DRAM) */
+   err = icc_link_create(node, TEGRA_ICC_EMEM);
+   if (err)
+   goto remove_nodes;
+
+   /* create External Memory node */
+   node = icc_node_create(TEGRA_ICC_EMEM);
+   err = PTR_ERR_OR_ZERO(node);
+   if (err)
+   goto remove_nodes;
+
+   node->name = "External Memory (DRAM)";
+   icc_node_add(node, >provider);
+
+   return 0;
+
+remove_nodes:
+   icc_nodes_remove(>provider);
+del_provider:
+   icc_provider_del(>provider);
+err_msg:
+   dev_err(emc->dev, "failed to initialize ICC: %d\n", err);
+
+   return err;
+}
+
 static int tegra_emc_probe(struct platform_device *pdev)
 {
struct platform_device *mc;
@@ -1343,6 +1452,7 @@ static int tegra_emc_probe(struct platform_device *pdev)
 
platform_set_drvdata(pdev, emc);
tegra_emc_debugfs_init(emc);
+   tegra_emc_interconnect_init(emc);
 
/*
 * Don't allow the kernel module to be unloaded. Unloading adds some
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 31/36] memory: tegra20-emc: Create tegra20-devfreq device

2020-08-17 Thread Dmitry Osipenko
The tegra20-devfreq driver provides memory frequency scaling functionality
and it uses EMC clock for the scaling. Since tegra20-devfreq is a software
driver, the device for the driver needs to be created manually. Let's do
it from EMC driver since it provides the clk rate-change functionality.

Signed-off-by: Dmitry Osipenko 
---
 drivers/memory/tegra/tegra20-emc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/memory/tegra/tegra20-emc.c 
b/drivers/memory/tegra/tegra20-emc.c
index 437d9d789941..e603cc0b0341 100644
--- a/drivers/memory/tegra/tegra20-emc.c
+++ b/drivers/memory/tegra/tegra20-emc.c
@@ -884,6 +884,9 @@ static int tegra_emc_probe(struct platform_device *pdev)
tegra_emc_debugfs_init(emc);
tegra_emc_interconnect_init(emc);
 
+   if (IS_ENABLED(CONFIG_ARM_TEGRA20_DEVFREQ))
+   platform_device_register_simple("tegra20-devfreq", -1, NULL, 0);
+
/*
 * Don't allow the kernel module to be unloaded. Unloading adds some
 * extra complexity which doesn't really worth the effort in a case of
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 06/36] memory: tegra124-emc: Make driver modular

2020-08-17 Thread Dmitry Osipenko
This patch adds modularization support to the Tegra124 EMC driver. Driver
now can be compiled as a loadable kernel module.

Signed-off-by: Dmitry Osipenko 
---
 drivers/clk/tegra/clk-tegra124-emc.c | 63 +---
 drivers/clk/tegra/clk-tegra124.c |  3 +-
 drivers/clk/tegra/clk.h  | 12 --
 drivers/memory/tegra/Kconfig |  2 +-
 drivers/memory/tegra/tegra124-emc.c  | 32 +-
 include/linux/clk/tegra.h| 11 +
 include/soc/tegra/emc.h  | 16 ---
 7 files changed, 73 insertions(+), 66 deletions(-)
 delete mode 100644 include/soc/tegra/emc.h

diff --git a/drivers/clk/tegra/clk-tegra124-emc.c 
b/drivers/clk/tegra/clk-tegra124-emc.c
index 745f9faa98d8..4d8b8f1ba7cd 100644
--- a/drivers/clk/tegra/clk-tegra124-emc.c
+++ b/drivers/clk/tegra/clk-tegra124-emc.c
@@ -11,7 +11,9 @@
 #include 
 #include 
 #include 
+#include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -21,10 +23,10 @@
 #include 
 
 #include 
-#include 
 
 #include "clk.h"
 
+#define CLK_BASE 0x60006000
 #define CLK_SOURCE_EMC 0x19c
 
 #define CLK_SOURCE_EMC_EMC_2X_CLK_DIVISOR_SHIFT 0
@@ -79,7 +81,9 @@ struct tegra_clk_emc {
 
int num_timings;
struct emc_timing *timings;
-   spinlock_t *lock;
+
+   tegra124_emc_prepare_timing_change_cb *prepare_timing_change;
+   tegra124_emc_complete_timing_change_cb *complete_timing_change;
 };
 
 /* Common clock framework callback implementations */
@@ -98,7 +102,7 @@ static unsigned long emc_recalc_rate(struct clk_hw *hw,
 */
parent_rate = clk_hw_get_rate(clk_hw_get_parent(hw));
 
-   val = readl(tegra->clk_regs + CLK_SOURCE_EMC);
+   val = readl(tegra->clk_regs);
div = val & CLK_SOURCE_EMC_EMC_2X_CLK_DIVISOR_MASK;
 
return parent_rate / (div + 2) * 2;
@@ -163,7 +167,7 @@ static u8 emc_get_parent(struct clk_hw *hw)
 
tegra = container_of(hw, struct tegra_clk_emc, hw);
 
-   val = readl(tegra->clk_regs + CLK_SOURCE_EMC);
+   val = readl(tegra->clk_regs);
 
return (val >> CLK_SOURCE_EMC_EMC_2X_CLK_SRC_SHIFT)
& CLK_SOURCE_EMC_EMC_2X_CLK_SRC_MASK;
@@ -204,7 +208,6 @@ static int emc_set_timing(struct tegra_clk_emc *tegra,
int err;
u8 div;
u32 car_value;
-   unsigned long flags = 0;
struct tegra_emc *emc = emc_ensure_emc_driver(tegra);
 
if (!emc)
@@ -241,13 +244,11 @@ static int emc_set_timing(struct tegra_clk_emc *tegra,
 
div = timing->parent_rate / (timing->rate / 2) - 2;
 
-   err = tegra_emc_prepare_timing_change(emc, timing->rate);
+   err = tegra->prepare_timing_change(emc, timing->rate);
if (err)
return err;
 
-   spin_lock_irqsave(tegra->lock, flags);
-
-   car_value = readl(tegra->clk_regs + CLK_SOURCE_EMC);
+   car_value = readl(tegra->clk_regs);
 
car_value &= ~CLK_SOURCE_EMC_EMC_2X_CLK_SRC(~0);
car_value |= CLK_SOURCE_EMC_EMC_2X_CLK_SRC(timing->parent_index);
@@ -255,11 +256,9 @@ static int emc_set_timing(struct tegra_clk_emc *tegra,
car_value &= ~CLK_SOURCE_EMC_EMC_2X_CLK_DIVISOR(~0);
car_value |= CLK_SOURCE_EMC_EMC_2X_CLK_DIVISOR(div);
 
-   writel(car_value, tegra->clk_regs + CLK_SOURCE_EMC);
-
-   spin_unlock_irqrestore(tegra->lock, flags);
+   writel(car_value, tegra->clk_regs);
 
-   tegra_emc_complete_timing_change(emc, timing->rate);
+   tegra->complete_timing_change(emc, timing->rate);
 
clk_hw_reparent(>hw, __clk_get_hw(timing->parent));
clk_disable_unprepare(tegra->prev_parent);
@@ -473,12 +472,15 @@ static const struct clk_ops tegra_clk_emc_ops = {
.get_parent = emc_get_parent,
 };
 
-struct clk *tegra_clk_register_emc(void __iomem *base, struct device_node *np,
-  spinlock_t *lock)
+struct clk *
+tegra124_clk_register_emc(struct device_node *emc_np,
+ tegra124_emc_prepare_timing_change_cb *prep_cb,
+ tegra124_emc_complete_timing_change_cb *complete_cb)
 {
struct tegra_clk_emc *tegra;
struct clk_init_data init;
struct device_node *node;
+   struct resource res;
u32 node_ram_code;
struct clk *clk;
int err;
@@ -487,12 +489,21 @@ struct clk *tegra_clk_register_emc(void __iomem *base, 
struct device_node *np,
if (!tegra)
return ERR_PTR(-ENOMEM);
 
-   tegra->clk_regs = base;
-   tegra->lock = lock;
+   res.start = CLK_BASE + CLK_SOURCE_EMC;
+   res.end = res.start + 3;
+   res.flags = IORESOURCE_MEM;
 
-   tegra->num_timings = 0;
+   tegra->clk_regs = ioremap(res.start, resource_size());
+   if (!tegra->clk_regs) {
+   pr_err("failed to map CLK_SOURCE_EMC\n");
+   return ERR_PTR(-EINVAL);
+   

[PATCH RESEND v12 1/4] drm/panel: Add helper for reading DT rotation

2020-08-17 Thread Dmitry Osipenko
From: Derek Basehore 

This adds a helper function for reading the rotation (panel
orientation) from the device tree.

Reviewed-by: Sam Ravnborg 
Tested-by: Dmitry Osipenko 
Signed-off-by: Derek Basehore 
Signed-off-by: Dmitry Osipenko 
---
 drivers/gpu/drm/drm_panel.c | 43 +
 include/drm/drm_panel.h |  9 
 2 files changed, 52 insertions(+)

diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index 8c7bac85a793..5557c75301f1 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -300,6 +300,49 @@ struct drm_panel *of_drm_find_panel(const struct 
device_node *np)
return ERR_PTR(-EPROBE_DEFER);
 }
 EXPORT_SYMBOL(of_drm_find_panel);
+
+/**
+ * of_drm_get_panel_orientation - look up the orientation of the panel through
+ * the "rotation" binding from a device tree node
+ * @np: device tree node of the panel
+ * @orientation: orientation enum to be filled in
+ *
+ * Looks up the rotation of a panel in the device tree. The orientation of the
+ * panel is expressed as a property name "rotation" in the device tree. The
+ * rotation in the device tree is counter clockwise.
+ *
+ * Return: 0 when a valid rotation value (0, 90, 180, or 270) is read or the
+ * rotation property doesn't exist. -EERROR otherwise.
+ */
+int of_drm_get_panel_orientation(const struct device_node *np,
+enum drm_panel_orientation *orientation)
+{
+   int rotation, ret;
+
+   ret = of_property_read_u32(np, "rotation", );
+   if (ret == -EINVAL) {
+   /* Don't return an error if there's no rotation property. */
+   *orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
+   return 0;
+   }
+
+   if (ret < 0)
+   return ret;
+
+   if (rotation == 0)
+   *orientation = DRM_MODE_PANEL_ORIENTATION_NORMAL;
+   else if (rotation == 90)
+   *orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP;
+   else if (rotation == 180)
+   *orientation = DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
+   else if (rotation == 270)
+   *orientation = DRM_MODE_PANEL_ORIENTATION_LEFT_UP;
+   else
+   return -EINVAL;
+
+   return 0;
+}
+EXPORT_SYMBOL(of_drm_get_panel_orientation);
 #endif
 
 #if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index 6193cb555acc..781c735f0f9b 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -35,6 +35,8 @@ struct drm_device;
 struct drm_panel;
 struct display_timing;
 
+enum drm_panel_orientation;
+
 /**
  * struct drm_panel_funcs - perform operations on a given panel
  *
@@ -191,11 +193,18 @@ int drm_panel_get_modes(struct drm_panel *panel, struct 
drm_connector *connector
 
 #if defined(CONFIG_OF) && defined(CONFIG_DRM_PANEL)
 struct drm_panel *of_drm_find_panel(const struct device_node *np);
+int of_drm_get_panel_orientation(const struct device_node *np,
+enum drm_panel_orientation *orientation);
 #else
 static inline struct drm_panel *of_drm_find_panel(const struct device_node *np)
 {
return ERR_PTR(-ENODEV);
 }
+static inline int of_drm_get_panel_orientation(const struct device_node *np,
+   enum drm_panel_orientation *orientation)
+{
+   return -ENODEV;
+}
 #endif
 
 #if IS_ENABLED(CONFIG_DRM_PANEL) && (IS_BUILTIN(CONFIG_BACKLIGHT_CLASS_DEVICE) 
|| \
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 30/36] memory: tegra20-emc: Register as interconnect provider

2020-08-17 Thread Dmitry Osipenko
Now memory controller is a memory interconnection provider. This allows us
to use interconnect API in order to change memory configuration.

Signed-off-by: Dmitry Osipenko 
---
 drivers/memory/tegra/tegra20-emc.c | 110 +
 1 file changed, 110 insertions(+)

diff --git a/drivers/memory/tegra/tegra20-emc.c 
b/drivers/memory/tegra/tegra20-emc.c
index 89e077d797e7..437d9d789941 100644
--- a/drivers/memory/tegra/tegra20-emc.c
+++ b/drivers/memory/tegra/tegra20-emc.c
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -148,6 +149,7 @@ struct emc_timing {
 struct tegra_emc {
struct device *dev;
struct notifier_block clk_nb;
+   struct icc_provider provider;
struct clk *clk;
void __iomem *regs;
 
@@ -661,6 +663,113 @@ static void tegra_emc_debugfs_init(struct tegra_emc *emc)
emc, _emc_debug_max_rate_fops);
 }
 
+static inline struct tegra_emc *
+to_tegra_emc_provider(struct icc_provider *provider)
+{
+   return container_of(provider, struct tegra_emc, provider);
+}
+
+static struct icc_node *
+emc_of_icc_xlate(struct of_phandle_args *spec, void *data)
+{
+   struct icc_provider *provider = data;
+   struct icc_node *node;
+
+   /* External Memory is the only possible ICC route */
+   list_for_each_entry(node, >nodes, node_list) {
+   if (node->id == TEGRA_ICC_EMEM)
+   return node;
+   }
+
+   return ERR_PTR(-EINVAL);
+}
+
+static int emc_icc_set(struct icc_node *src, struct icc_node *dst)
+{
+   struct tegra_emc *emc = to_tegra_emc_provider(dst->provider);
+   unsigned long long rate = icc_units_to_bps(dst->avg_bw);
+   unsigned int dram_data_bus_width_bytes = 4;
+   unsigned int ddr = 2;
+   int err;
+
+   do_div(rate, ddr * dram_data_bus_width_bytes);
+   rate = min_t(u64, rate, U32_MAX);
+
+   err = clk_set_min_rate(emc->clk, rate);
+   if (err)
+   return err;
+
+   err = clk_set_rate(emc->clk, rate);
+   if (err)
+   return err;
+
+   return 0;
+}
+
+static int emc_icc_aggregate(struct icc_node *node,
+u32 tag, u32 avg_bw, u32 peak_bw,
+u32 *agg_avg, u32 *agg_peak)
+{
+   *agg_avg = min((u64)avg_bw + (*agg_avg), (u64)U32_MAX);
+   *agg_peak = max(*agg_peak, peak_bw);
+
+   return 0;
+}
+
+static int tegra_emc_interconnect_init(struct tegra_emc *emc)
+{
+   struct icc_node *node;
+   int err;
+
+   /* older device-trees don't have interconnect properties */
+   if (!of_find_property(emc->dev->of_node, "#interconnect-cells", NULL))
+   return 0;
+
+   emc->provider.dev = emc->dev;
+   emc->provider.set = emc_icc_set;
+   emc->provider.data = >provider;
+   emc->provider.xlate = emc_of_icc_xlate;
+   emc->provider.aggregate = emc_icc_aggregate;
+
+   err = icc_provider_add(>provider);
+   if (err)
+   goto err_msg;
+
+   /* create External Memory Controller node */
+   node = icc_node_create(TEGRA_ICC_EMC);
+   err = PTR_ERR_OR_ZERO(node);
+   if (err)
+   goto del_provider;
+
+   node->name = "External Memory Controller";
+   icc_node_add(node, >provider);
+
+   /* link External Memory Controller to External Memory (DRAM) */
+   err = icc_link_create(node, TEGRA_ICC_EMEM);
+   if (err)
+   goto remove_nodes;
+
+   /* create External Memory node */
+   node = icc_node_create(TEGRA_ICC_EMEM);
+   err = PTR_ERR_OR_ZERO(node);
+   if (err)
+   goto remove_nodes;
+
+   node->name = "External Memory (DRAM)";
+   icc_node_add(node, >provider);
+
+   return 0;
+
+remove_nodes:
+   icc_nodes_remove(>provider);
+del_provider:
+   icc_provider_del(>provider);
+err_msg:
+   dev_err(emc->dev, "failed to initialize ICC: %d\n", err);
+
+   return err;
+}
+
 static int tegra_emc_init_mc_timings(struct tegra_emc *emc)
 {
struct tegra_mc_timing *timing;
@@ -773,6 +882,7 @@ static int tegra_emc_probe(struct platform_device *pdev)
 
platform_set_drvdata(pdev, emc);
tegra_emc_debugfs_init(emc);
+   tegra_emc_interconnect_init(emc);
 
/*
 * Don't allow the kernel module to be unloaded. Unloading adds some
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH RESEND v10 0/4] Support DRM bridges on NVIDIA Tegra

2020-08-17 Thread Dmitry Osipenko
Hello,

This series adds initial support for the DRM bridges to NVIDIA Tegra DRM
driver. This is required by newer device-trees where we model the LVDS
encoder bridge properly. In particular this series is needed in order to
light up display panels of recently merged Acer A500 and Nexus 7 devices.

Changelog:

v10: - No changes. Patches missed v5.9 kernel, re-sending for v5.10.
   @Thierry, please pick up this series into linux-next or let me
   know what needs to be changed, thanks in advance!

v9: - Dropped the of-graph/drm-of patches from this series because they
  are now factored out into a standalone series [1].

  [1] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=186813

- The "drm/panel-simple: Add missing connector type for some panels"
  patch of v8 was already applied.

v8: - The new of_graph_get_local_port() helper is replaced with the
  of_graph_presents(), which simply checks the graph presence in a
  given DT node. Thank to Laurent Pinchart for the suggestion!

- The of_graph_get_local_port() is still there, but now it isn't a public
  function anymore. In the review to v7 Laurent Pinchart suggested that
  the function's doc-comments and name could be improved and I implemented
  these suggestions in v8.

- A day ago I discovered that devm_drm_panel_bridge_add() requires
  panel to have connector type to be properly set, otherwise function
  rejects panels with the incomplete description. So, I checked what
  LVDS panels are used on Tegra and fixed the missing connector types
  in this new patch:

drm/panel-simple: Add missing connector type for some panels

v7: - Removed the obscure unused structs (which GCC doesn't detect, but CLANG
  does) in the patch "Wrap directly-connected panel into DRM bridge",
  which was reported by kernel test robot for v6.

v6: - Added r-b and acks from Rob Herring and Sam Ravnborg.

- Rebased on a recent linux-next, patches now apply without fuzz.

v5: - Added new patches that make drm_of_find_panel_or_bridge() more usable
  if graph isn't defined in a device-tree:

of_graph: add of_graph_get_local_port()
drm/of: Make drm_of_find_panel_or_bridge() to check graph's presence

- Updated "Support DRM bridges" patch to use drm_of_find_panel_or_bridge()
  directly and added WARN_ON(output->panel || output->bridge) sanity-check.

- Added new "Wrap directly-connected panel into DRM bridge" patch, as
  was suggested by Laurent Pinchart.

v4: - Following review comments that were made by Laurent Pinchart to the v3,
  we now create and use the "bridge connector".

v3: - Following recommendation from Sam Ravnborg, the new bridge attachment
  model is now being used, i.e. we ask bridge to *not* create a connector
  using the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag.

- The bridge is now created only for the RGB (LVDS) output, and only
  when necessary. For now we don't need bridges for HDMI or DSI outputs.

- I noticed that we're leaking OF node in the panel's error code path,
  this is fixed now by the new patch "Don't leak OF node on error".

v2: - Added the new "rgb: Don't register connector if bridge is used"
  patch, which hides the unused connector provided by the Tegra DRM
  driver when bridge is used, since bridge provides its own connector
  to us.


Dmitry Osipenko (4):
  drm/tegra: output: Don't leak OF node on error
  drm/tegra: output: Support DRM bridges
  drm/tegra: output: rgb: Support LVDS encoder bridge
  drm/tegra: output: rgb: Wrap directly-connected panel into DRM bridge

 drivers/gpu/drm/tegra/drm.h|   2 +
 drivers/gpu/drm/tegra/output.c |  21 +--
 drivers/gpu/drm/tegra/rgb.c| 102 +
 3 files changed, 72 insertions(+), 53 deletions(-)

-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH RESEND v10 4/4] drm/tegra: output: rgb: Wrap directly-connected panel into DRM bridge

2020-08-17 Thread Dmitry Osipenko
Currently Tegra DRM driver manually manages display panel, but this
management could be moved out into DRM core if we'll wrap panel into
DRM bridge. This patch wraps RGB panel into a DRM bridge and removes
manual handling of the panel from the RGB output code.

Suggested-by: Laurent Pinchart 
Acked-by: Sam Ravnborg 
Signed-off-by: Dmitry Osipenko 
---
 drivers/gpu/drm/tegra/rgb.c | 70 ++---
 1 file changed, 18 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/tegra/rgb.c b/drivers/gpu/drm/tegra/rgb.c
index 9a7024ec96bc..4142a56ca764 100644
--- a/drivers/gpu/drm/tegra/rgb.c
+++ b/drivers/gpu/drm/tegra/rgb.c
@@ -8,7 +8,6 @@
 
 #include 
 #include 
-#include 
 #include 
 
 #include "drm.h"
@@ -86,45 +85,13 @@ static void tegra_dc_write_regs(struct tegra_dc *dc,
tegra_dc_writel(dc, table[i].value, table[i].offset);
 }
 
-static const struct drm_connector_funcs tegra_rgb_connector_funcs = {
-   .reset = drm_atomic_helper_connector_reset,
-   .detect = tegra_output_connector_detect,
-   .fill_modes = drm_helper_probe_single_connector_modes,
-   .destroy = tegra_output_connector_destroy,
-   .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
-   .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
-
-static enum drm_mode_status
-tegra_rgb_connector_mode_valid(struct drm_connector *connector,
-  struct drm_display_mode *mode)
-{
-   /*
-* FIXME: For now, always assume that the mode is okay. There are
-* unresolved issues with clk_round_rate(), which doesn't always
-* reliably report whether a frequency can be set or not.
-*/
-   return MODE_OK;
-}
-
-static const struct drm_connector_helper_funcs 
tegra_rgb_connector_helper_funcs = {
-   .get_modes = tegra_output_connector_get_modes,
-   .mode_valid = tegra_rgb_connector_mode_valid,
-};
-
 static void tegra_rgb_encoder_disable(struct drm_encoder *encoder)
 {
struct tegra_output *output = encoder_to_output(encoder);
struct tegra_rgb *rgb = to_rgb(output);
 
-   if (output->panel)
-   drm_panel_disable(output->panel);
-
tegra_dc_write_regs(rgb->dc, rgb_disable, ARRAY_SIZE(rgb_disable));
tegra_dc_commit(rgb->dc);
-
-   if (output->panel)
-   drm_panel_unprepare(output->panel);
 }
 
 static void tegra_rgb_encoder_enable(struct drm_encoder *encoder)
@@ -133,9 +100,6 @@ static void tegra_rgb_encoder_enable(struct drm_encoder 
*encoder)
struct tegra_rgb *rgb = to_rgb(output);
u32 value;
 
-   if (output->panel)
-   drm_panel_prepare(output->panel);
-
tegra_dc_write_regs(rgb->dc, rgb_enable, ARRAY_SIZE(rgb_enable));
 
value = DE_SELECT_ACTIVE | DE_CONTROL_NORMAL;
@@ -157,9 +121,6 @@ static void tegra_rgb_encoder_enable(struct drm_encoder 
*encoder)
tegra_dc_writel(rgb->dc, value, DC_DISP_SHIFT_CLOCK_OPTIONS);
 
tegra_dc_commit(rgb->dc);
-
-   if (output->panel)
-   drm_panel_enable(output->panel);
 }
 
 static int
@@ -278,6 +239,23 @@ int tegra_dc_rgb_init(struct drm_device *drm, struct 
tegra_dc *dc)
drm_encoder_helper_add(>encoder,
   _rgb_encoder_helper_funcs);
 
+   /*
+* Wrap directly-connected panel into DRM bridge in order to let
+* DRM core to handle panel for us.
+*/
+   if (output->panel) {
+   output->bridge = devm_drm_panel_bridge_add(output->dev,
+  output->panel);
+   if (IS_ERR(output->bridge)) {
+   dev_err(output->dev,
+   "failed to wrap panel into bridge: %pe\n",
+   output->bridge);
+   return PTR_ERR(output->bridge);
+   }
+
+   output->panel = NULL;
+   }
+
/*
 * Tegra devices that have LVDS panel utilize LVDS encoder bridge
 * for converting up to 28 LCD LVTTL lanes into 5/4 LVDS lanes that
@@ -292,8 +270,7 @@ int tegra_dc_rgb_init(struct drm_device *drm, struct 
tegra_dc *dc)
 * Newer device-trees utilize LVDS encoder bridge, which provides
 * us with a connector and handles the display panel.
 *
-* For older device-trees we fall back to our own connector and use
-* nvidia,panel phandle.
+* For older device-trees we wrapped panel into the panel-bridge.
 */
if (output->bridge) {
err = drm_bridge_attach(>encoder, output->bridge,
@@ -313,17 +290,6 @@ int tegra_dc_rgb_init(struct drm_device *drm, struct 
tegra_dc *dc)
}
 
drm_connector_attach_encoder(connector, >encoder);
-   } else {
- 

[PATCH v5 15/36] PM / devfreq: tegra30: Add error messages to tegra_devfreq_target()

2020-08-17 Thread Dmitry Osipenko
It's useful to now when something goes wrong instead of failing silently,
so let's add error messages to tegra_devfreq_target() to prevent situation
where it fails silently.

Acked-by: Chanwoo Choi 
Signed-off-by: Dmitry Osipenko 
---
 drivers/devfreq/tegra30-devfreq.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/devfreq/tegra30-devfreq.c 
b/drivers/devfreq/tegra30-devfreq.c
index 6c2f56b34535..a0d6f628aaa4 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -641,12 +641,16 @@ static int tegra_devfreq_target(struct device *dev, 
unsigned long *freq,
dev_pm_opp_put(opp);
 
err = clk_set_min_rate(tegra->emc_clock, rate * KHZ);
-   if (err)
+   if (err) {
+   dev_err(dev, "Failed to set min rate: %d\n", err);
return err;
+   }
 
err = clk_set_rate(tegra->emc_clock, 0);
-   if (err)
+   if (err) {
+   dev_err(dev, "Failed to set rate: %d\n", err);
goto restore_min_rate;
+   }
 
return 0;
 
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 09/36] memory: tegra20-emc: Initialize MC timings

2020-08-17 Thread Dmitry Osipenko
We're going to add interconnect support to the EMC driver. Once this
support will be added, the Tegra20 devfreq driver will no longer be
able to use clk_round_rate(emc) for building up OPP table. It's quite
handy that struct tegra_mc contains memory timings which could be used
by the devfreq drivers instead of the clk rate-rounding. The tegra_mc
timings are populated by the MC driver only for Tegra30+ SoCs, hence
the Tegra20 EMC could populate timings by itself.

Signed-off-by: Dmitry Osipenko 
---
 drivers/memory/tegra/Kconfig   |  2 +-
 drivers/memory/tegra/tegra20-emc.c | 54 ++
 2 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
index c1cad4ce6251..5bf75b316a2f 100644
--- a/drivers/memory/tegra/Kconfig
+++ b/drivers/memory/tegra/Kconfig
@@ -10,7 +10,7 @@ config TEGRA_MC
 config TEGRA20_EMC
tristate "NVIDIA Tegra20 External Memory Controller driver"
default y
-   depends on ARCH_TEGRA_2x_SOC
+   depends on TEGRA_MC && ARCH_TEGRA_2x_SOC
help
  This driver is for the External Memory Controller (EMC) found on
  Tegra20 chips. The EMC controls the external DRAM on the board.
diff --git a/drivers/memory/tegra/tegra20-emc.c 
b/drivers/memory/tegra/tegra20-emc.c
index 5aa3a1da2975..a02ffc09c39e 100644
--- a/drivers/memory/tegra/tegra20-emc.c
+++ b/drivers/memory/tegra/tegra20-emc.c
@@ -15,12 +15,15 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 
 #include 
 
+#include "mc.h"
+
 #define EMC_INTSTATUS  0x000
 #define EMC_INTMASK0x004
 #define EMC_DBG0x008
@@ -650,6 +653,45 @@ static void tegra_emc_debugfs_init(struct tegra_emc *emc)
emc, _emc_debug_max_rate_fops);
 }
 
+static int tegra_emc_init_mc_timings(struct tegra_emc *emc)
+{
+   struct tegra_mc_timing *timing;
+   struct platform_device *pdev;
+   struct device_node *np;
+   struct tegra_mc *mc;
+   unsigned int i;
+
+   if (!emc->num_timings)
+   return 0;
+
+   np = of_find_compatible_node(NULL, NULL, "nvidia,tegra20-mc-gart");
+   if (!np)
+   return -ENOENT;
+
+   pdev = of_find_device_by_node(np);
+   of_node_put(np);
+   if (!pdev)
+   return -ENOENT;
+
+   mc = platform_get_drvdata(pdev);
+   if (!mc)
+   return -EPROBE_DEFER;
+
+   /* shouldn't happen */
+   WARN_ON(mc->num_timings);
+   WARN_ON(mc->timings);
+
+   mc->timings = devm_kcalloc(emc->dev, emc->num_timings, sizeof(*timing),
+  GFP_KERNEL);
+   if (!mc->timings)
+   return -ENOMEM;
+
+   for (i = 0; i < emc->num_timings; i++, mc->num_timings++)
+   mc->timings[i].rate = emc->timings[i].rate;
+
+   return 0;
+}
+
 static int tegra_emc_probe(struct platform_device *pdev)
 {
struct device_node *np;
@@ -721,6 +763,18 @@ static int tegra_emc_probe(struct platform_device *pdev)
goto unset_cb;
}
 
+   /*
+* Only Tegra30+ SoCs are having Memory Controller timings initialized
+* by the MC driver. For Tegra20 we need to populate the MC timings
+* from here. The MC timings will be used by the Tegra20 devfreq driver.
+*/
+   err = tegra_emc_init_mc_timings(emc);
+   if (err) {
+   dev_err(>dev, "failed to initialize mc timings: %d\n",
+   err);
+   goto unset_cb;
+   }
+
platform_set_drvdata(pdev, emc);
tegra_emc_debugfs_init(emc);
 
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 29/36] memory: tegra20-emc: Continue probing if timings are missing in device-tree

2020-08-17 Thread Dmitry Osipenko
EMC driver will become mandatory after turning it into interconnect
provider because interconnect users, like display controller driver, will
fail to probe using newer device-trees that have interconnect properties.
Thus make EMC driver to probe even if timings are missing in device-tree.

Signed-off-by: Dmitry Osipenko 
---
 drivers/memory/tegra/tegra20-emc.c | 34 ++
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/drivers/memory/tegra/tegra20-emc.c 
b/drivers/memory/tegra/tegra20-emc.c
index db6a4bcb92fb..89e077d797e7 100644
--- a/drivers/memory/tegra/tegra20-emc.c
+++ b/drivers/memory/tegra/tegra20-emc.c
@@ -386,6 +386,11 @@ tegra_emc_find_node_by_ram_code(struct device *dev)
u32 value, ram_code;
int err;
 
+   if (of_get_child_count(dev->of_node) == 0) {
+   dev_info(dev, "device-tree doesn't have memory timings\n");
+   return NULL;
+   }
+
if (!of_property_read_bool(dev->of_node, "nvidia,use-ram-code"))
return of_node_get(dev->of_node);
 
@@ -454,6 +459,9 @@ static long emc_round_rate(unsigned long rate,
struct tegra_emc *emc = arg;
unsigned int i;
 
+   if (!emc->num_timings)
+   return clk_get_rate(emc->clk);
+
min_rate = min(min_rate, emc->timings[emc->num_timings - 1].rate);
 
for (i = 0; i < emc->num_timings; i++) {
@@ -698,13 +706,6 @@ static int tegra_emc_probe(struct platform_device *pdev)
struct tegra_emc *emc;
int irq, err;
 
-   /* driver has nothing to do in a case of memory timing absence */
-   if (of_get_child_count(pdev->dev.of_node) == 0) {
-   dev_info(>dev,
-"EMC device tree node doesn't have memory timings\n");
-   return 0;
-   }
-
irq = platform_get_irq(pdev, 0);
if (irq < 0) {
dev_err(>dev, "interrupt not specified\n");
@@ -712,23 +713,20 @@ static int tegra_emc_probe(struct platform_device *pdev)
return irq;
}
 
-   np = tegra_emc_find_node_by_ram_code(>dev);
-   if (!np)
-   return -EINVAL;
-
emc = devm_kzalloc(>dev, sizeof(*emc), GFP_KERNEL);
-   if (!emc) {
-   of_node_put(np);
+   if (!emc)
return -ENOMEM;
-   }
 
emc->clk_nb.notifier_call = tegra_emc_clk_change_notify;
emc->dev = >dev;
 
-   err = tegra_emc_load_timings_from_dt(emc, np);
-   of_node_put(np);
-   if (err)
-   return err;
+   np = tegra_emc_find_node_by_ram_code(>dev);
+   if (np) {
+   err = tegra_emc_load_timings_from_dt(emc, np);
+   of_node_put(np);
+   if (err)
+   return err;
+   }
 
emc->regs = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(emc->regs))
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 07/36] memory: tegra124-emc: Use devm_platform_ioremap_resource

2020-08-17 Thread Dmitry Osipenko
Utilize that relatively new helper which makes code a bit cleaner.

Signed-off-by: Dmitry Osipenko 
---
 drivers/memory/tegra/tegra124-emc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/memory/tegra/tegra124-emc.c 
b/drivers/memory/tegra/tegra124-emc.c
index 93e9835e9761..8d7fc3fb00bb 100644
--- a/drivers/memory/tegra/tegra124-emc.c
+++ b/drivers/memory/tegra/tegra124-emc.c
@@ -1194,7 +1194,6 @@ static int tegra_emc_probe(struct platform_device *pdev)
struct platform_device *mc;
struct device_node *np;
struct tegra_emc *emc;
-   struct resource *res;
u32 ram_code;
int err;
 
@@ -1204,8 +1203,7 @@ static int tegra_emc_probe(struct platform_device *pdev)
 
emc->dev = >dev;
 
-   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   emc->regs = devm_ioremap_resource(>dev, res);
+   emc->regs = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(emc->regs))
return PTR_ERR(emc->regs);
 
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 19/36] dt-bindings: memory: tegra20: emc: Document new interconnect property

2020-08-17 Thread Dmitry Osipenko
External memory controller is interconnected with memory controller and
with external memory. Document new interconnect property which turns
external memory controller into interconnect provider.

Acked-by: Rob Herring 
Signed-off-by: Dmitry Osipenko 
---
 .../bindings/memory-controllers/nvidia,tegra20-emc.txt  | 2 ++
 1 file changed, 2 insertions(+)

diff --git 
a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt 
b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt
index add95367640b..f51da7662de4 100644
--- 
a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt
+++ 
b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt
@@ -12,6 +12,7 @@ Properties:
   irrespective of ram-code configuration.
 - interrupts : Should contain EMC General interrupt.
 - clocks : Should contain EMC clock.
+- #interconnect-cells : Should be 0.
 
 Child device nodes describe the memory settings for different configurations 
and clock rates.
 
@@ -20,6 +21,7 @@ Example:
memory-controller@7000f400 {
#address-cells = < 1 >;
#size-cells = < 0 >;
+   #interconnect-cells = < 0 >;
compatible = "nvidia,tegra20-emc";
reg = <0x7000f4000 0x200>;
interrupts = <0 78 0x04>;
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 05/36] memory: tegra30-emc: Make driver modular

2020-08-17 Thread Dmitry Osipenko
This patch adds modularization support to the Tegra30 EMC driver. Driver
now can be compiled as a loadable kernel module.

Signed-off-by: Dmitry Osipenko 
---
 drivers/memory/tegra/Kconfig   |  2 +-
 drivers/memory/tegra/mc.c  |  3 +++
 drivers/memory/tegra/tegra30-emc.c | 17 -
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
index 7e0e1ef87763..bd453de9d446 100644
--- a/drivers/memory/tegra/Kconfig
+++ b/drivers/memory/tegra/Kconfig
@@ -18,7 +18,7 @@ config TEGRA20_EMC
  external memory.
 
 config TEGRA30_EMC
-   bool "NVIDIA Tegra30 External Memory Controller driver"
+   tristate "NVIDIA Tegra30 External Memory Controller driver"
default y
depends on TEGRA_MC && ARCH_TEGRA_3x_SOC
help
diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
index ec8403557ed4..772aa021b5f6 100644
--- a/drivers/memory/tegra/mc.c
+++ b/drivers/memory/tegra/mc.c
@@ -6,6 +6,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -298,6 +299,7 @@ int tegra_mc_write_emem_configuration(struct tegra_mc *mc, 
unsigned long rate)
 
return 0;
 }
+EXPORT_SYMBOL_GPL(tegra_mc_write_emem_configuration);
 
 unsigned int tegra_mc_get_emem_device_count(struct tegra_mc *mc)
 {
@@ -309,6 +311,7 @@ unsigned int tegra_mc_get_emem_device_count(struct tegra_mc 
*mc)
 
return dram_count;
 }
+EXPORT_SYMBOL_GPL(tegra_mc_get_emem_device_count);
 
 static int load_one_timing(struct tegra_mc *mc,
   struct tegra_mc_timing *timing,
diff --git a/drivers/memory/tegra/tegra30-emc.c 
b/drivers/memory/tegra/tegra30-emc.c
index e448a55cb812..f3082964cfb6 100644
--- a/drivers/memory/tegra/tegra30-emc.c
+++ b/drivers/memory/tegra/tegra30-emc.c
@@ -1343,6 +1343,13 @@ static int tegra_emc_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, emc);
tegra_emc_debugfs_init(emc);
 
+   /*
+* Don't allow the kernel module to be unloaded. Unloading adds some
+* extra complexity which doesn't really worth the effort in a case of
+* this driver.
+*/
+   try_module_get(THIS_MODULE);
+
return 0;
 
 unset_cb:
@@ -1393,6 +1400,7 @@ static const struct of_device_id tegra_emc_of_match[] = {
{ .compatible = "nvidia,tegra30-emc", },
{},
 };
+MODULE_DEVICE_TABLE(of, tegra_emc_of_match);
 
 static struct platform_driver tegra_emc_driver = {
.probe = tegra_emc_probe,
@@ -1403,9 +1411,8 @@ static struct platform_driver tegra_emc_driver = {
.suppress_bind_attrs = true,
},
 };
+module_platform_driver(tegra_emc_driver);
 
-static int __init tegra_emc_init(void)
-{
-   return platform_driver_register(_emc_driver);
-}
-subsys_initcall(tegra_emc_init);
+MODULE_AUTHOR("Dmitry Osipenko ");
+MODULE_DESCRIPTION("NVIDIA Tegra30 EMC driver");
+MODULE_LICENSE("GPL v2");
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 21/36] dt-bindings: memory: tegra30: emc: Document new interconnect property

2020-08-17 Thread Dmitry Osipenko
External memory controller is interconnected with memory controller and
with external memory. Document new interconnect property which turns
external memory controller into interconnect provider.

Acked-by: Rob Herring 
Signed-off-by: Dmitry Osipenko 
---
 .../bindings/memory-controllers/nvidia,tegra30-emc.yaml | 6 ++
 1 file changed, 6 insertions(+)

diff --git 
a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml 
b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml
index 112bae2fcbbd..c243986db420 100644
--- 
a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml
+++ 
b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml
@@ -31,6 +31,9 @@ properties:
   interrupts:
 maxItems: 1
 
+  "#interconnect-cells":
+const: 0
+
   nvidia,memory-controller:
 $ref: /schemas/types.yaml#/definitions/phandle
 description:
@@ -214,6 +217,7 @@ required:
   - interrupts
   - clocks
   - nvidia,memory-controller
+  - "#interconnect-cells"
 
 additionalProperties: false
 
@@ -227,6 +231,8 @@ examples:
 
 nvidia,memory-controller = <>;
 
+#interconnect-cells = <0>;
+
 emc-timings-1 {
 nvidia,ram-code = <1>;
 
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 34/36] drm/tegra: dc: Support memory bandwidth management

2020-08-17 Thread Dmitry Osipenko
Display controller (DC) performs isochronous memory transfers, and thus,
has a requirement for a minimum memory bandwidth that shall be fulfilled,
otherwise framebuffer data can't be fetched fast enough and this results
in a DC's data-FIFO underflow that follows by a visual corruption.

The Memory Controller drivers provide facility for memory bandwidth
management via interconnect API. This patch wires up the interconnect
API support to the DC driver.

Signed-off-by: Dmitry Osipenko 
---
 drivers/gpu/drm/tegra/Kconfig |   1 +
 drivers/gpu/drm/tegra/dc.c| 271 +-
 drivers/gpu/drm/tegra/dc.h|   8 +
 drivers/gpu/drm/tegra/drm.c   |  19 +++
 drivers/gpu/drm/tegra/plane.c |   1 +
 drivers/gpu/drm/tegra/plane.h |   4 +-
 6 files changed, 300 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tegra/Kconfig b/drivers/gpu/drm/tegra/Kconfig
index 5043dcaf1cf9..1650a448eabd 100644
--- a/drivers/gpu/drm/tegra/Kconfig
+++ b/drivers/gpu/drm/tegra/Kconfig
@@ -9,6 +9,7 @@ config DRM_TEGRA
select DRM_MIPI_DSI
select DRM_PANEL
select TEGRA_HOST1X
+   select INTERCONNECT
select IOMMU_IOVA
select CEC_CORE if CEC_NOTIFIER
help
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 9a0b3240bc58..850fbcebefc2 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -525,6 +525,136 @@ static void tegra_dc_setup_window(struct tegra_plane 
*plane,
tegra_plane_setup_blending(plane, window);
 }
 
+static unsigned long
+tegra_plane_memory_bandwidth(struct drm_plane_state *state,
+struct tegra_dc_window *window,
+unsigned int num,
+unsigned int denum)
+{
+   struct tegra_plane_state *tegra_state;
+   struct drm_crtc_state *crtc_state;
+   const struct drm_format_info *fmt;
+   struct tegra_dc_window win;
+   unsigned long long bandwidth;
+   unsigned int bpp_plane;
+   unsigned int bpp;
+   unsigned int mul;
+   unsigned int i;
+
+   if (!state->fb || !state->visible)
+   return 0;
+
+   crtc_state = drm_atomic_get_new_crtc_state(state->state, state->crtc);
+   tegra_state = to_tegra_plane_state(state);
+
+   if (!window)
+   window = 
+
+   window->src.w = drm_rect_width(>src) >> 16;
+   window->src.h = drm_rect_height(>src) >> 16;
+   window->dst.w = drm_rect_width(>dst);
+   window->dst.h = drm_rect_height(>dst);
+   window->tiling = tegra_state->tiling;
+
+   fmt = state->fb->format;
+
+   /*
+* Note that real memory bandwidth vary depending on format and
+* memory layout, we are not taking that into account because small
+* estimation error isn't important since bandwidth is rounded up
+* anyway.
+*/
+   for (i = 0, bpp = 0; i < fmt->num_planes; i++) {
+   bpp_plane = fmt->cpp[i] * 8;
+
+   /*
+* Sub-sampling is relevant for chroma planes only and vertical
+* readouts are not cached, hence only horizontal sub-sampling
+* matters.
+*/
+   if (i > 0)
+   bpp_plane /= fmt->hsub;
+
+   bpp += bpp_plane;
+   }
+
+   /*
+* Horizontal downscale takes extra bandwidth which roughly depends
+* on the scaled width.
+*/
+   if (window->src.w > window->dst.w)
+   mul = (window->src.w - window->dst.w) * bpp / 2048 + 1;
+   else
+   mul = 1;
+
+   /*
+* Ignore cursor window if its width is small enough such that
+* data-prefetch FIFO will easily help to overcome temporal memory
+* pressure.
+*
+* Window A has a 128bit x 128 deep read FIFO, while windows B/C
+* have a 128bit x 64 deep read FIFO.
+*
+* This allows us to not overestimate memory frequency requirement.
+* Even if it will happen that cursor gets a temporal underflow, this
+* won't be fatal.
+*/
+   if (state->plane->type == DRM_PLANE_TYPE_CURSOR &&
+   mul == 1 && window->src.w * bpp <= 128 * 16)
+   return 0;
+
+   /* mode.clock in kHz, bandwidth in kbit/s */
+   bandwidth = kbps_to_icc(crtc_state->mode.clock * bpp * mul);
+
+   /* the requested bandwidth should be higher than required */
+   bandwidth *= num;
+   do_div(bandwidth, denum);
+
+   return min_t(u64, bandwidth, ULONG_MAX);
+}
+
+static unsigned long
+tegra20_plane_memory_bandwidth(struct drm_plane_state *state)
+{
+   return tegra_plane_memory_bandwidth(state, NULL, 29, 10);
+}
+
+static unsigned long
+tegra30_plane_memory_bandwidth(struct drm_plane_state *state)
+{
+   struct te

[PATCH v5 28/36] memory: tegra20-emc: Use devm_platform_ioremap_resource

2020-08-17 Thread Dmitry Osipenko
Utilize that relatively new helper which makes code a bit cleaner.

Signed-off-by: Dmitry Osipenko 
---
 drivers/memory/tegra/tegra20-emc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/memory/tegra/tegra20-emc.c 
b/drivers/memory/tegra/tegra20-emc.c
index a02ffc09c39e..db6a4bcb92fb 100644
--- a/drivers/memory/tegra/tegra20-emc.c
+++ b/drivers/memory/tegra/tegra20-emc.c
@@ -696,7 +696,6 @@ static int tegra_emc_probe(struct platform_device *pdev)
 {
struct device_node *np;
struct tegra_emc *emc;
-   struct resource *res;
int irq, err;
 
/* driver has nothing to do in a case of memory timing absence */
@@ -731,8 +730,7 @@ static int tegra_emc_probe(struct platform_device *pdev)
if (err)
return err;
 
-   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   emc->regs = devm_ioremap_resource(>dev, res);
+   emc->regs = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(emc->regs))
return PTR_ERR(emc->regs);
 
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 12/36] PM / devfreq: tegra20: Use MC timings for building OPP table

2020-08-17 Thread Dmitry Osipenko
The clk_round_rate() won't be usable for building OPP table once
interconnect support will be added to the EMC driver because that CLK API
function limits the rounded rate based on the clk rate that is imposed by
active clk-users, and thus, the rounding won't work as expected if
interconnect will set the minimum EMC clock rate before devfreq driver is
loaded. The struct tegra_mc contains memory timings which could be used by
the devfreq driver for building up OPP table instead of rounding clock
rate, this patch implements this idea.

Signed-off-by: Dmitry Osipenko 
---
 drivers/devfreq/tegra20-devfreq.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/devfreq/tegra20-devfreq.c 
b/drivers/devfreq/tegra20-devfreq.c
index 6469dc69c5e0..a985f24098f5 100644
--- a/drivers/devfreq/tegra20-devfreq.c
+++ b/drivers/devfreq/tegra20-devfreq.c
@@ -123,8 +123,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
 {
struct tegra_devfreq *tegra;
struct tegra_mc *mc;
-   unsigned long max_rate;
-   unsigned long rate;
+   unsigned int i;
int err;
 
mc = tegra_get_memory_controller();
@@ -135,6 +134,11 @@ static int tegra_devfreq_probe(struct platform_device 
*pdev)
return err;
}
 
+   if (!mc->num_timings) {
+   dev_info(>dev, "memory controller has no timings\n");
+   return -ENODEV;
+   }
+
tegra = devm_kzalloc(>dev, sizeof(*tegra), GFP_KERNEL);
if (!tegra)
return -ENOMEM;
@@ -151,12 +155,8 @@ static int tegra_devfreq_probe(struct platform_device 
*pdev)
 
tegra->regs = mc->regs;
 
-   max_rate = clk_round_rate(tegra->emc_clock, ULONG_MAX);
-
-   for (rate = 0; rate <= max_rate; rate++) {
-   rate = clk_round_rate(tegra->emc_clock, rate);
-
-   err = dev_pm_opp_add(>dev, rate, 0);
+   for (i = 0; i < mc->num_timings; i++) {
+   err = dev_pm_opp_add(>dev, mc->timings[i].rate, 0);
if (err) {
dev_err(>dev, "failed to add opp: %d\n", err);
goto remove_opps;
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 32/36] memory: tegra30-emc: Continue probing if timings are missing in device-tree

2020-08-17 Thread Dmitry Osipenko
EMC driver will become mandatory after turning it into interconnect
provider because interconnect users, like display controller driver, will
fail to probe using newer device-trees that have interconnect properties.
Thus make EMC driver to probe even if timings are missing in device-tree.

Signed-off-by: Dmitry Osipenko 
---
 drivers/memory/tegra/tegra30-emc.c | 29 +++--
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/memory/tegra/tegra30-emc.c 
b/drivers/memory/tegra/tegra30-emc.c
index f3082964cfb6..0cd39bf5f393 100644
--- a/drivers/memory/tegra/tegra30-emc.c
+++ b/drivers/memory/tegra/tegra30-emc.c
@@ -988,6 +988,11 @@ static struct device_node 
*emc_find_node_by_ram_code(struct device *dev)
u32 value, ram_code;
int err;
 
+   if (of_get_child_count(dev->of_node) == 0) {
+   dev_info(dev, "device-tree doesn't have memory timings\n");
+   return NULL;
+   }
+
ram_code = tegra_read_ram_code();
 
for_each_child_of_node(dev->of_node, np) {
@@ -1057,6 +1062,9 @@ static long emc_round_rate(unsigned long rate,
struct tegra_emc *emc = arg;
unsigned int i;
 
+   if (!emc->num_timings)
+   return clk_get_rate(emc->clk);
+
min_rate = min(min_rate, emc->timings[emc->num_timings - 1].rate);
 
for (i = 0; i < emc->num_timings; i++) {
@@ -1263,12 +1271,6 @@ static int tegra_emc_probe(struct platform_device *pdev)
struct tegra_emc *emc;
int err;
 
-   if (of_get_child_count(pdev->dev.of_node) == 0) {
-   dev_info(>dev,
-"device-tree node doesn't have memory timings\n");
-   return -ENODEV;
-   }
-
np = of_parse_phandle(pdev->dev.of_node, "nvidia,memory-controller", 0);
if (!np) {
dev_err(>dev, "could not get memory controller node\n");
@@ -1280,10 +1282,6 @@ static int tegra_emc_probe(struct platform_device *pdev)
if (!mc)
return -ENOENT;
 
-   np = emc_find_node_by_ram_code(>dev);
-   if (!np)
-   return -EINVAL;
-
emc = devm_kzalloc(>dev, sizeof(*emc), GFP_KERNEL);
if (!emc) {
of_node_put(np);
@@ -1297,10 +1295,13 @@ static int tegra_emc_probe(struct platform_device *pdev)
emc->clk_nb.notifier_call = emc_clk_change_notify;
emc->dev = >dev;
 
-   err = emc_load_timings_from_dt(emc, np);
-   of_node_put(np);
-   if (err)
-   return err;
+   np = emc_find_node_by_ram_code(>dev);
+   if (np) {
+   err = emc_load_timings_from_dt(emc, np);
+   of_node_put(np);
+   if (err)
+   return err;
+   }
 
emc->regs = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(emc->regs))
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 17/36] PM / devfreq: tegra20: Relax Kconfig dependency

2020-08-17 Thread Dmitry Osipenko
The Tegra EMC driver now could be compiled as a loadable kernel module.
Currently devfreq driver depends on the EMC/MC drivers in Kconfig, and
thus, devfreq is forced to be a kernel module if EMC is compiled as a
module. This build dependency could be relaxed since devfreq driver
checks MC/EMC presence on probe, allowing kernel configuration where
devfreq is a built-in driver and EMC driver is a loadable module.
This change puts Tegra20 devfreq Kconfig entry on a par with the Tegra30
devfreq entry.

Acked-by: Chanwoo Choi 
Signed-off-by: Dmitry Osipenko 
---
 drivers/devfreq/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
index 37dc40d1fcfb..0ee36ae2fa79 100644
--- a/drivers/devfreq/Kconfig
+++ b/drivers/devfreq/Kconfig
@@ -123,7 +123,7 @@ config ARM_TEGRA_DEVFREQ
 
 config ARM_TEGRA20_DEVFREQ
tristate "NVIDIA Tegra20 DEVFREQ Driver"
-   depends on (TEGRA_MC && TEGRA20_EMC) || COMPILE_TEST
+   depends on ARCH_TEGRA_2x_SOC || COMPILE_TEST
depends on COMMON_CLK
select DEVFREQ_GOV_SIMPLE_ONDEMAND
help
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 02/36] clk: tegra: Remove Memory Controller lock

2020-08-17 Thread Dmitry Osipenko
The shared Memory Controller lock isn't needed since the time when
Memory Clock was made read-only. The lock could be removed safely now.
Hence let's remove it, this will help a tad to make further patches
cleaner.

Signed-off-by: Dmitry Osipenko 
---
 drivers/clk/tegra/clk-divider.c  | 4 ++--
 drivers/clk/tegra/clk-tegra114.c | 6 ++
 drivers/clk/tegra/clk-tegra124.c | 7 ++-
 drivers/clk/tegra/clk-tegra20.c  | 3 +--
 drivers/clk/tegra/clk-tegra30.c  | 3 +--
 drivers/clk/tegra/clk.h  | 2 +-
 6 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/clk/tegra/clk-divider.c b/drivers/clk/tegra/clk-divider.c
index 38daf483ddf1..56adb01638cc 100644
--- a/drivers/clk/tegra/clk-divider.c
+++ b/drivers/clk/tegra/clk-divider.c
@@ -177,10 +177,10 @@ static const struct clk_div_table mc_div_table[] = {
 };
 
 struct clk *tegra_clk_register_mc(const char *name, const char *parent_name,
- void __iomem *reg, spinlock_t *lock)
+ void __iomem *reg)
 {
return clk_register_divider_table(NULL, name, parent_name,
  CLK_IS_CRITICAL,
  reg, 16, 1, CLK_DIVIDER_READ_ONLY,
- mc_div_table, lock);
+ mc_div_table, NULL);
 }
diff --git a/drivers/clk/tegra/clk-tegra114.c b/drivers/clk/tegra/clk-tegra114.c
index bc9e47a4cb60..ca8d9737d301 100644
--- a/drivers/clk/tegra/clk-tegra114.c
+++ b/drivers/clk/tegra/clk-tegra114.c
@@ -134,7 +134,6 @@ static DEFINE_SPINLOCK(pll_d_lock);
 static DEFINE_SPINLOCK(pll_d2_lock);
 static DEFINE_SPINLOCK(pll_u_lock);
 static DEFINE_SPINLOCK(pll_re_lock);
-static DEFINE_SPINLOCK(emc_lock);
 
 static struct div_nmp pllxc_nmp = {
.divm_shift = 0,
@@ -1050,10 +1049,9 @@ static __init void tegra114_periph_clk_init(void __iomem 
*clk_base,
   ARRAY_SIZE(mux_pllmcp_clkm),
   CLK_SET_RATE_NO_REPARENT,
   clk_base + CLK_SOURCE_EMC,
-  29, 3, 0, _lock);
+  29, 3, 0, NULL);
 
-   clk = tegra_clk_register_mc("mc", "emc_mux", clk_base + CLK_SOURCE_EMC,
-   _lock);
+   clk = tegra_clk_register_mc("mc", "emc_mux", clk_base + CLK_SOURCE_EMC);
clks[TEGRA114_CLK_MC] = clk;
 
clk = tegra_clk_register_periph_gate("mipi-cal", "clk_m", 0, clk_base,
diff --git a/drivers/clk/tegra/clk-tegra124.c b/drivers/clk/tegra/clk-tegra124.c
index e931319dcc9d..0c956e14b9ca 100644
--- a/drivers/clk/tegra/clk-tegra124.c
+++ b/drivers/clk/tegra/clk-tegra124.c
@@ -126,7 +126,6 @@ static DEFINE_SPINLOCK(pll_d_lock);
 static DEFINE_SPINLOCK(pll_e_lock);
 static DEFINE_SPINLOCK(pll_re_lock);
 static DEFINE_SPINLOCK(pll_u_lock);
-static DEFINE_SPINLOCK(emc_lock);
 static DEFINE_SPINLOCK(sor0_lock);
 
 /* possible OSC frequencies in Hz */
@@ -1050,8 +1049,7 @@ static __init void tegra124_periph_clk_init(void __iomem 
*clk_base,
 periph_clk_enb_refcnt);
clks[TEGRA124_CLK_DSIB] = clk;
 
-   clk = tegra_clk_register_mc("mc", "emc", clk_base + CLK_SOURCE_EMC,
-   _lock);
+   clk = tegra_clk_register_mc("mc", "emc", clk_base + CLK_SOURCE_EMC);
clks[TEGRA124_CLK_MC] = clk;
 
/* cml0 */
@@ -1518,8 +1516,7 @@ static void __init tegra124_132_clock_init_post(struct 
device_node *np)
  tegra124_reset_deassert);
tegra_add_of_provider(np, of_clk_src_onecell_get);
 
-   clks[TEGRA124_CLK_EMC] = tegra_clk_register_emc(clk_base, np,
-   _lock);
+   clks[TEGRA124_CLK_EMC] = tegra_clk_register_emc(clk_base, np, NULL);
 
tegra_register_devclks(devclks, ARRAY_SIZE(devclks));
 
diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
index 3efc651b42e3..2f8b6de4198f 100644
--- a/drivers/clk/tegra/clk-tegra20.c
+++ b/drivers/clk/tegra/clk-tegra20.c
@@ -802,8 +802,7 @@ static void __init tegra20_periph_clk_init(void)
 
clks[TEGRA20_CLK_EMC] = clk;
 
-   clk = tegra_clk_register_mc("mc", "emc", clk_base + CLK_SOURCE_EMC,
-   NULL);
+   clk = tegra_clk_register_mc("mc", "emc", clk_base + CLK_SOURCE_EMC);
clks[TEGRA20_CLK_MC] = clk;
 
/* dsi */
diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
index 37244a7e68c2..88e8c485f8ae 100644
--- a/drivers/clk/tegra/clk-tegra30.c
+++ b/drivers/clk/tegra/clk-tegra30.c
@@ -1042,8 +1042,7 @@ static void __init tegra30_periph_clk_init(void)
 
clks[TEGRA30_CLK_EMC] = clk;
 
-   clk = tegra_clk_r

[PATCH v5 27/36] memory: tegra-mc: Register as interconnect provider

2020-08-17 Thread Dmitry Osipenko
Now memory controller is a memory interconnection provider. This allows us
to use interconnect API in order to change memory configuration.

Signed-off-by: Dmitry Osipenko 
---
 drivers/memory/tegra/Kconfig |   1 +
 drivers/memory/tegra/mc.c| 118 +++
 drivers/memory/tegra/mc.h|   8 +++
 include/soc/tegra/mc.h   |   3 +
 4 files changed, 130 insertions(+)

diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
index 5bf75b316a2f..7055fdef2c32 100644
--- a/drivers/memory/tegra/Kconfig
+++ b/drivers/memory/tegra/Kconfig
@@ -3,6 +3,7 @@ config TEGRA_MC
bool "NVIDIA Tegra Memory Controller support"
default y
depends on ARCH_TEGRA
+   select INTERCONNECT
help
  This driver supports the Memory Controller (MC) hardware found on
  NVIDIA Tegra SoCs.
diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
index 772aa021b5f6..46759ddaa3c9 100644
--- a/drivers/memory/tegra/mc.c
+++ b/drivers/memory/tegra/mc.c
@@ -594,6 +594,122 @@ static __maybe_unused irqreturn_t tegra20_mc_irq(int irq, 
void *data)
return IRQ_HANDLED;
 }
 
+static int tegra_mc_icc_set(struct icc_node *src, struct icc_node *dst)
+{
+   /*
+* The plan is to populate this function with a latency allowness
+* programming sometime later, for now this a dummy callback.
+*/
+   return 0;
+}
+
+static int tegra_mc_icc_aggregate(struct icc_node *node,
+ u32 tag, u32 avg_bw, u32 peak_bw,
+ u32 *agg_avg, u32 *agg_peak)
+{
+   *agg_avg = min((u64)avg_bw + (*agg_avg), (u64)U32_MAX);
+   *agg_peak = max(*agg_peak, peak_bw);
+
+   return 0;
+}
+
+/*
+ * Memory Controller (MC) has few Memory Clients that are issuing memory
+ * bandwidth allocation requests to the MC interconnect provider. The MC
+ * provider aggregates the requests and then sends the aggregated request
+ * up to the External Memory Controller (EMC) interconnect provider which
+ * re-configures hardware interface to External Memory (EMEM) in accordance
+ * to the required bandwidth. Each MC interconnect node represents an
+ * individual Memory Client.
+ *
+ * Memory interconnect topology:
+ *
+ *   ++
+ * ++||
+ * | TEXSRD +--->+|
+ * ++||
+ *   ||+-++--+
+ *...| MC +--->+ EMC +--->+ EMEM |
+ *   ||+-++--+
+ * ++||
+ * | DISP.. +--->+|
+ * ++||
+ *   ++
+ */
+static int tegra_mc_interconnect_setup(struct tegra_mc *mc)
+{
+   struct icc_onecell_data *data;
+   struct icc_node *node;
+   unsigned int num_nodes;
+   unsigned int i;
+   int err;
+
+   /* older device-trees don't have interconnect properties */
+   if (!of_find_property(mc->dev->of_node, "#interconnect-cells", NULL))
+   return 0;
+
+   num_nodes = mc->soc->num_clients;
+
+   data = devm_kzalloc(mc->dev, struct_size(data, nodes, num_nodes),
+   GFP_KERNEL);
+   if (!data)
+   return -ENOMEM;
+
+   mc->provider.dev = mc->dev;
+   mc->provider.set = tegra_mc_icc_set;
+   mc->provider.data = data;
+   mc->provider.xlate = of_icc_xlate_onecell;
+   mc->provider.aggregate = tegra_mc_icc_aggregate;
+
+   err = icc_provider_add(>provider);
+   if (err)
+   goto err_msg;
+
+   /* create Memory Controller node */
+   node = icc_node_create(TEGRA_ICC_MC);
+   err = PTR_ERR_OR_ZERO(node);
+   if (err)
+   goto del_provider;
+
+   node->name = "Memory Controller";
+   icc_node_add(node, >provider);
+
+   /* link Memory Controller to External Memory Controller */
+   err = icc_link_create(node, TEGRA_ICC_EMC);
+   if (err)
+   goto remove_nodes;
+
+   for (i = 0; i < num_nodes; i++) {
+   /* create MC client node */
+   node = icc_node_create(mc->soc->clients[i].id);
+   err = PTR_ERR_OR_ZERO(node);
+   if (err)
+   goto remove_nodes;
+
+   node->name = mc->soc->clients[i].name;
+   icc_node_add(node, >provider);
+
+   /* link Memory Client to Memory Controller */
+   err = icc_link_create(node, TEGRA_ICC_MC);
+   if (err)
+   goto remove_nodes;
+
+   data->nodes[i] = node;
+   }
+   data->num_nodes = num_nodes;
+
+   return 0;
+
+remove_nodes:
+   icc_nodes_remove(>provider);
+del_provider:
+   icc_provider_del(>provider);
+err_msg:
+   dev_err(mc->dev, "failed to initialize ICC: %d\n", err);
+
+   return err;
+}
+
 static int 

[PATCH RESEND v12 0/4] Panel rotation patches

2020-08-17 Thread Dmitry Osipenko
Hello!

This series adds support for display panel's DT rotation property. It's a
continuation of the work that was initially started by Derek Basehore for
the panel driver that is used by some Mediatek device [1]. I picked up the
Derek's patches and added my t-b and r-b tags to them, I also added
rotation support to the panel-lvds and panel-simple drivers.

We need the rotation support for the Nexus 7 tablet device which is now
supported by the upstream kernel, the device has display panel mounted
upside-down and it uses panel-lvds [2].

[1] https://lkml.org/lkml/2020/3/5/1119
[2] 
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm/boot/dts/tegra30-asus-nexus7-grouper-common.dtsi?#n1036

Changelog:

v12: - No code changes. The v11 missed v5.9 release, re-sending patches
   for the v5.10 kernel. Please review and apply patches to linux-next,
   thanks in advance!

v11: - This series is factored out from this patchset [3] because these
   patches do not have hard dependency on the Tegra DRM patches and
   it should be nicer to review and apply the properly grouped patches.

 - Initially [3] only touched the panel-lvds driver and Emil Velikov
   suggested that it will be better to support more panels in the review
   comments to [3]. So I included the Derek's patch for the BOE panel
   and added rotation support to the panel-simple driver. I tested that
   panel-lvds and panel-simple work properly with the rotated panel using
   the Opentegra Xorg driver [4] and Wayland Weston [5].

 - The panel-lvds driver now prints a error message if rotation property
   fails to be parsed.

[3] https://lore.kernel.org/lkml/20200614200121.14147-1-dig...@gmail.com/
[4] 
https://github.com/grate-driver/xf86-video-opentegra/commit/28eb20a3959bbe5bc3a3b67e55977093fd5114ca
[5] https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/315

Derek Basehore (2):
  drm/panel: Add helper for reading DT rotation
  drm/panel: Read panel orientation for BOE TV101WUM-NL6

Dmitry Osipenko (2):
  drm/panel: lvds: Read panel orientation
  drm/panel-simple: Read panel orientation

 drivers/gpu/drm/drm_panel.c   | 43 +++
 .../gpu/drm/panel/panel-boe-tv101wum-nl6.c|  6 +++
 drivers/gpu/drm/panel/panel-lvds.c| 10 +
 drivers/gpu/drm/panel/panel-simple.c  | 11 +
 include/drm/drm_panel.h   |  9 
 5 files changed, 79 insertions(+)

-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 16/36] PM / devfreq: tegra20: Adjust clocks conversion ratio and polling interval

2020-08-17 Thread Dmitry Osipenko
The current conversion ratio results in a higher frequency than needed,
that is not very actual now since the Display Controller driver got
support for memory bandwidth management and hence memory frequency can
go lower now without bad consequences. Since memory freq now goes to a
lower rates, the responsiveness of interactive applications become worse
due to a quite high polling interval value that is currently set to 500ms.
Changing polling interval to 30ms results in a good responsiveness of the
system.

Acked-by: Chanwoo Choi 
Signed-off-by: Dmitry Osipenko 
---
 drivers/devfreq/tegra20-devfreq.c | 14 +-
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/devfreq/tegra20-devfreq.c 
b/drivers/devfreq/tegra20-devfreq.c
index bc43284996de..ebfc5ac9e5c3 100644
--- a/drivers/devfreq/tegra20-devfreq.c
+++ b/drivers/devfreq/tegra20-devfreq.c
@@ -79,16 +79,12 @@ static int tegra_devfreq_get_dev_status(struct device *dev,
 
/*
 * EMC_COUNT returns number of memory events, that number is lower
-* than the number of clocks. Conversion ratio of 1/8 results in a
-* bit higher bandwidth than actually needed, it is good enough for
-* the time being because drivers don't support requesting minimum
-* needed memory bandwidth yet.
-*
-* TODO: adjust the ratio value once relevant drivers will support
-* memory bandwidth management.
+* than the number of total EMC clocks over the sampling period.
+* The clocks number is converted to maximum possible number of
+* memory events using the ratio of 1/4.
 */
stat->busy_time = readl_relaxed(tegra->regs + MC_STAT_EMC_COUNT);
-   stat->total_time = readl_relaxed(tegra->regs + MC_STAT_EMC_CLOCKS) / 8;
+   stat->total_time = readl_relaxed(tegra->regs + MC_STAT_EMC_CLOCKS) / 4;
stat->current_frequency = clk_get_rate(tegra->emc_clock);
 
writel_relaxed(EMC_GATHER_CLEAR, tegra->regs + MC_STAT_CONTROL);
@@ -98,7 +94,7 @@ static int tegra_devfreq_get_dev_status(struct device *dev,
 }
 
 static struct devfreq_dev_profile tegra_devfreq_profile = {
-   .polling_ms = 500,
+   .polling_ms = 30,
.target = tegra_devfreq_target,
.get_dev_status = tegra_devfreq_get_dev_status,
 };
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 13/36] PM / devfreq: tegra30: Use MC timings for building OPP table

2020-08-17 Thread Dmitry Osipenko
The clk_round_rate() won't be usable for building OPP table once
interconnect support will be added to the EMC driver because that CLK API
function limits the rounded rate based on the clk rate that is imposed by
active clk-users, and thus, the rounding won't work as expected if
interconnect will set the minimum EMC clock rate before devfreq driver is
loaded. The struct tegra_mc contains memory timings which could be used by
the devfreq driver for building up OPP table instead of rounding clock
rate, this patch implements this idea.

Signed-off-by: Dmitry Osipenko 
---
 drivers/devfreq/tegra30-devfreq.c | 93 +--
 1 file changed, 65 insertions(+), 28 deletions(-)

diff --git a/drivers/devfreq/tegra30-devfreq.c 
b/drivers/devfreq/tegra30-devfreq.c
index 423dd35c95b3..6c2f56b34535 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -19,6 +19,8 @@
 #include 
 #include 
 
+#include 
+
 #include "governor.h"
 
 #define ACTMON_GLB_STATUS  0x0
@@ -153,6 +155,18 @@ struct tegra_devfreq_device {
unsigned long target_freq;
 };
 
+struct tegra_devfreq_soc_data {
+   const char *mc_compatible;
+};
+
+static const struct tegra_devfreq_soc_data tegra30_soc = {
+   .mc_compatible = "nvidia,tegra30-mc",
+};
+
+static const struct tegra_devfreq_soc_data tegra124_soc = {
+   .mc_compatible = "nvidia,tegra124-mc",
+};
+
 struct tegra_devfreq {
struct devfreq  *devfreq;
 
@@ -771,15 +785,49 @@ static struct devfreq_governor tegra_devfreq_governor = {
.interrupt_driven = true,
 };
 
+static struct tegra_mc *tegra_get_memory_controller(const char *compatible)
+{
+   struct platform_device *pdev;
+   struct device_node *np;
+   struct tegra_mc *mc;
+
+   np = of_find_compatible_node(NULL, NULL, compatible);
+   if (!np)
+   return ERR_PTR(-ENOENT);
+
+   pdev = of_find_device_by_node(np);
+   of_node_put(np);
+   if (!pdev)
+   return ERR_PTR(-ENODEV);
+
+   mc = platform_get_drvdata(pdev);
+   if (!mc)
+   return ERR_PTR(-EPROBE_DEFER);
+
+   return mc;
+}
+
 static int tegra_devfreq_probe(struct platform_device *pdev)
 {
+   const struct tegra_devfreq_soc_data *soc_data;
struct tegra_devfreq_device *dev;
struct tegra_devfreq *tegra;
struct devfreq *devfreq;
+   struct tegra_mc *mc;
unsigned int i;
-   long rate;
int err;
 
+   soc_data = of_device_get_match_data(>dev);
+
+   mc = tegra_get_memory_controller(soc_data->mc_compatible);
+   if (IS_ERR(mc))
+   return PTR_ERR(mc);
+
+   if (!mc->num_timings) {
+   dev_info(>dev, "memory controller has no timings\n");
+   return -ENODEV;
+   }
+
tegra = devm_kzalloc(>dev, sizeof(*tegra), GFP_KERNEL);
if (!tegra)
return -ENOMEM;
@@ -825,6 +873,20 @@ static int tegra_devfreq_probe(struct platform_device 
*pdev)
return err;
}
 
+   for (i = 0; i < mc->num_timings; i++) {
+   /*
+* Memory Controller timings are sorted in ascending clock
+* rate order, so the last timing will be the max freq.
+*/
+   tegra->max_freq = mc->timings[i].rate / KHZ;
+
+   err = dev_pm_opp_add(>dev, tegra->max_freq, 0);
+   if (err) {
+   dev_err(>dev, "Failed to add OPP: %d\n", err);
+   goto remove_opps;
+   }
+   }
+
reset_control_assert(tegra->reset);
 
err = clk_prepare_enable(tegra->clock);
@@ -836,37 +898,12 @@ static int tegra_devfreq_probe(struct platform_device 
*pdev)
 
reset_control_deassert(tegra->reset);
 
-   rate = clk_round_rate(tegra->emc_clock, ULONG_MAX);
-   if (rate < 0) {
-   dev_err(>dev, "Failed to round clock rate: %ld\n", rate);
-   return rate;
-   }
-
-   tegra->max_freq = rate / KHZ;
-
for (i = 0; i < ARRAY_SIZE(actmon_device_configs); i++) {
dev = tegra->devices + i;
dev->config = actmon_device_configs + i;
dev->regs = tegra->regs + dev->config->offset;
}
 
-   for (rate = 0; rate <= tegra->max_freq * KHZ; rate++) {
-   rate = clk_round_rate(tegra->emc_clock, rate);
-
-   if (rate < 0) {
-   dev_err(>dev,
-   "Failed to round clock rate: %ld\n", rate);
-   err = rate;
-   goto remove_opps;
-   }
-
-   err = dev_pm_opp_add(>dev, rate / KHZ, 0);
-   if (err) {
-   dev_err(>

[PATCH v5 26/36] ARM: tegra: Add interconnect properties to Tegra30 device-tree

2020-08-17 Thread Dmitry Osipenko
Add interconnect properties to the memory controller, external memory
controller and the display controller nodes in order to describe hardware
interconnection.

Signed-off-by: Dmitry Osipenko 
---
 arch/arm/boot/dts/tegra30.dtsi | 23 ++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
index aeae8c092d41..b28360d3c039 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -210,6 +210,15 @@ dc@5420 {
 
nvidia,head = <0>;
 
+   interconnects = < TEGRA30_MC_DISPLAY0A >,
+   < TEGRA30_MC_DISPLAY0B >,
+   < TEGRA30_MC_DISPLAY0C >,
+   < TEGRA30_MC_DISPLAY1B >;
+   interconnect-names = "wina",
+"winb",
+"winc",
+"cursor";
+
rgb {
status = "disabled";
};
@@ -229,6 +238,15 @@ dc@5424 {
 
nvidia,head = <1>;
 
+   interconnects = < TEGRA30_MC_DISPLAY0AB >,
+   < TEGRA30_MC_DISPLAY0BB >,
+   < TEGRA30_MC_DISPLAY0CB >,
+   < TEGRA30_MC_DISPLAY1BB >;
+   interconnect-names = "wina",
+"winb",
+"winc",
+"cursor";
+
rgb {
status = "disabled";
};
@@ -748,15 +766,18 @@ mc: memory-controller@7000f000 {
 
#iommu-cells = <1>;
#reset-cells = <1>;
+   #interconnect-cells = <1>;
};
 
-   memory-controller@7000f400 {
+   emc: memory-controller@7000f400 {
compatible = "nvidia,tegra30-emc";
reg = <0x7000f400 0x400>;
interrupts = ;
clocks = <_car TEGRA30_CLK_EMC>;
 
nvidia,memory-controller = <>;
+
+   #interconnect-cells = <0>;
};
 
fuse@7000f800 {
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 24/36] dt-bindings: memory: tegra30: Add memory client IDs

2020-08-17 Thread Dmitry Osipenko
Each memory client have a unique hardware ID, this patch adds these IDs.

Acked-by: Rob Herring 
Signed-off-by: Dmitry Osipenko 
---
 include/dt-bindings/memory/tegra30-mc.h | 67 +
 1 file changed, 67 insertions(+)

diff --git a/include/dt-bindings/memory/tegra30-mc.h 
b/include/dt-bindings/memory/tegra30-mc.h
index 169f005fbc78..930f708aca17 100644
--- a/include/dt-bindings/memory/tegra30-mc.h
+++ b/include/dt-bindings/memory/tegra30-mc.h
@@ -41,4 +41,71 @@
 #define TEGRA30_MC_RESET_VDE   16
 #define TEGRA30_MC_RESET_VI17
 
+#define TEGRA30_MC_PTCR0
+#define TEGRA30_MC_DISPLAY0A   1
+#define TEGRA30_MC_DISPLAY0AB  2
+#define TEGRA30_MC_DISPLAY0B   3
+#define TEGRA30_MC_DISPLAY0BB  4
+#define TEGRA30_MC_DISPLAY0C   5
+#define TEGRA30_MC_DISPLAY0CB  6
+#define TEGRA30_MC_DISPLAY1B   7
+#define TEGRA30_MC_DISPLAY1BB  8
+#define TEGRA30_MC_EPPUP   9
+#define TEGRA30_MC_G2PR10
+#define TEGRA30_MC_G2SR11
+#define TEGRA30_MC_MPEUNIFBR   12
+#define TEGRA30_MC_VIRUV   13
+#define TEGRA30_MC_AFIR14
+#define TEGRA30_MC_AVPCARM7R   15
+#define TEGRA30_MC_DISPLAYHC   16
+#define TEGRA30_MC_DISPLAYHCB  17
+#define TEGRA30_MC_FDCDRD  18
+#define TEGRA30_MC_FDCDRD2 19
+#define TEGRA30_MC_G2DR20
+#define TEGRA30_MC_HDAR21
+#define TEGRA30_MC_HOST1XDMAR  22
+#define TEGRA30_MC_HOST1XR 23
+#define TEGRA30_MC_IDXSRD  24
+#define TEGRA30_MC_IDXSRD2 25
+#define TEGRA30_MC_MPE_IPRED   26
+#define TEGRA30_MC_MPEAMEMRD   27
+#define TEGRA30_MC_MPECSRD 28
+#define TEGRA30_MC_PPCSAHBDMAR 29
+#define TEGRA30_MC_PPCSAHBSLVR 30
+#define TEGRA30_MC_SATAR   31
+#define TEGRA30_MC_TEXSRD  32
+#define TEGRA30_MC_TEXSRD2 33
+#define TEGRA30_MC_VDEBSEVR34
+#define TEGRA30_MC_VDEMBER 35
+#define TEGRA30_MC_VDEMCER 36
+#define TEGRA30_MC_VDETPER 37
+#define TEGRA30_MC_MPCORELPR   38
+#define TEGRA30_MC_MPCORER 39
+#define TEGRA30_MC_EPPU40
+#define TEGRA30_MC_EPPV41
+#define TEGRA30_MC_EPPY42
+#define TEGRA30_MC_MPEUNIFBW   43
+#define TEGRA30_MC_VIWSB   44
+#define TEGRA30_MC_VIWU45
+#define TEGRA30_MC_VIWV46
+#define TEGRA30_MC_VIWY47
+#define TEGRA30_MC_G2DW48
+#define TEGRA30_MC_AFIW49
+#define TEGRA30_MC_AVPCARM7W   50
+#define TEGRA30_MC_FDCDWR  51
+#define TEGRA30_MC_FDCDWR2 52
+#define TEGRA30_MC_HDAW53
+#define TEGRA30_MC_HOST1XW 54
+#define TEGRA30_MC_ISPW55
+#define TEGRA30_MC_MPCORELPW   56
+#define TEGRA30_MC_MPCOREW 57
+#define TEGRA30_MC_MPECSWR 58
+#define TEGRA30_MC_PPCSAHBDMAW 59
+#define TEGRA30_MC_PPCSAHBSLVW 60
+#define TEGRA30_MC_SATAW   61
+#define TEGRA30_MC_VDEBSEVW62
+#define TEGRA30_MC_VDEDBGW 63
+#define TEGRA30_MC_VDEMBEW 64
+#define TEGRA30_MC_VDETPMW 65
+
 #endif
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 10/36] PM / devfreq: tegra20: Silence deferred probe error

2020-08-17 Thread Dmitry Osipenko
Tegra EMC driver was turned into a regular kernel driver, it also could
be compiled as a loadable kernel module now. Hence EMC clock isn't
guaranteed to be available and clk_get("emc") may return -EPROBE_DEFER and
there is no good reason to spam KMSG with a error about missing EMC clock
in this case, so let's silence the deferred probe error.

Acked-by: Chanwoo Choi 
Signed-off-by: Dmitry Osipenko 
---
 drivers/devfreq/tegra20-devfreq.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/devfreq/tegra20-devfreq.c 
b/drivers/devfreq/tegra20-devfreq.c
index ff82bac9ee4e..6469dc69c5e0 100644
--- a/drivers/devfreq/tegra20-devfreq.c
+++ b/drivers/devfreq/tegra20-devfreq.c
@@ -141,9 +141,11 @@ static int tegra_devfreq_probe(struct platform_device 
*pdev)
 
/* EMC is a system-critical clock that is always enabled */
tegra->emc_clock = devm_clk_get(>dev, "emc");
-   if (IS_ERR(tegra->emc_clock)) {
-   err = PTR_ERR(tegra->emc_clock);
-   dev_err(>dev, "failed to get emc clock: %d\n", err);
+   err = PTR_ERR_OR_ZERO(tegra->emc_clock);
+   if (err) {
+   if (err != -EPROBE_DEFER)
+   dev_err(>dev, "failed to get emc clock: %d\n",
+   err);
return err;
}
 
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 03/36] clk: tegra: Export Tegra20 EMC kernel symbols

2020-08-17 Thread Dmitry Osipenko
We're going to modularize Tegra EMC drivers and some of the EMC clk driver
symbols need to be exported, let's export them.

Signed-off-by: Dmitry Osipenko 
---
 drivers/clk/tegra/clk-tegra20-emc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/clk/tegra/clk-tegra20-emc.c 
b/drivers/clk/tegra/clk-tegra20-emc.c
index 03bf0009a33c..dd74b8543bf1 100644
--- a/drivers/clk/tegra/clk-tegra20-emc.c
+++ b/drivers/clk/tegra/clk-tegra20-emc.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -235,6 +236,7 @@ void 
tegra20_clk_set_emc_round_callback(tegra20_clk_emc_round_cb *round_cb,
emc->cb_arg = cb_arg;
}
 }
+EXPORT_SYMBOL_GPL(tegra20_clk_set_emc_round_callback);
 
 bool tegra20_clk_emc_driver_available(struct clk_hw *emc_hw)
 {
@@ -291,3 +293,4 @@ int tegra20_clk_prepare_emc_mc_same_freq(struct clk 
*emc_clk, bool same)
 
return 0;
 }
+EXPORT_SYMBOL_GPL(tegra20_clk_prepare_emc_mc_same_freq);
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 00/36] Introduce memory interconnect for NVIDIA Tegra SoCs

2020-08-17 Thread Dmitry Osipenko
Hello,

This series brings initial support for memory interconnect to Tegra20 and
Tegra30 SoCs.

For the starter only display controllers are getting interconnect API
support, others could be supported later on. The display controllers
have the biggest demand for interconnect API right now because dynamic
memory frequency scaling can't be done safely without taking into account
bandwidth requirement from the displays.

Changelog:

v5: - The devfreq drivers now won't probe if memory timings aren't specified
  in a device-tree, like was suggested by Chanwoo Choi in a review comment
  to v4. Initially I wanted to always probe the driver even with a single
  fixed memory freq, but after a closer look turned out it can't be done
  easily for Tegra20 driver.

- The "interconnect: Relax requirement in of_icc_get_from_provider()"
  patch was already applied, hence one less patch in comparison to v4.

- Renamed display interconnect paths in accordance to the names that
  were used by Thierry Reding in one of his recent patches that supposed
  to update the Host1x's DT binding.

- Added acks from Chanwoo Choi.

- Added clarifying comment to tegra_mc_icc_set() about why it's a dummy
  function, this is done in a response to the review comment made by
  Georgi Djakov to v4.

v4: - All drivers that use interconnect API now select it in the Kconfig in
  order to properly express the build dependency.

- The IS_ENABLED(CONFIG_INTERCONNECT) is dropped now from all patches.

- Added MODULE_AUTHOR() to the modularized drivers, for completeness.

- Added missed TEGRA_MC Kconfig dependency for the Tegra20 EMC driver.

- Added more acks from Rob Herring that I accidentally missed to add in v3.

v3: - Added acks from Rob Herring that were given to some of the v2 patches.

- Specified name of the TRM documentation chapter in the patch
  "dt-bindings: host1x: Document new interconnect properties", which was
  suggested by Rob Herring in the review comment to v2.

- Added patches that allow EMC drivers to be compiled as a loadable kernel
  modules. This came up during of the v2 review when Georgi Djakov pointed
  out that interconnect-core could be compiled as a kernel module. Please
  note that the Tegra124 EMC driver is compile-tested only, I don't have
  Tegra124 HW.

- In the review comment to [1] Stephen Boyd suggested that it will be
  better not to make changes to clk API, which was needed in order to
  avoid clashing of the interconnect driver with the devfreq in regards
  to memory clk-rate rounding.

  [1] 
https://patchwork.ozlabs.org/project/linux-tegra/patch/20200330231617.17079-3-dig...@gmail.com/

  Stephen Boyd suggested that instead we should provide OPP table via DT.
  I tried to investigate whether this could be done and turned out
  it's a bit complicated. Technically it should be doable, but:

1. For now we don't fully support voltage scaling of the CORE regulator
   and so OPP table in the DT isn't really needed today. We can
   generate table from the memory timings, which is what Tegra devfreq
   drivers already do.

2. The OPP table should be defined in the DT for the Memory Controller
   node and then its usage somehow should be shared by both interconnect
   and devfreq drivers. It's not obvious what's the best way to do it.

  So, it will be much better to postpone the DT OPP table addition
  until these questions are resolved. We can infer OPPs from the
  memory timings and we could get the memory rates from the memory
  driver directly, avoiding the problems induced by the clk API usage.
  This idea is implemented in v3, see these patches:

PM / devfreq: tegra20: Use MC timings for building OPP table
PM / devfreq: tegra30: Use MC timings for building OPP table

v2: - Instead of a single dma-mem interconnect path, the paths are now
  defined per memory client.

- The EMC provider now uses #interconnect-cells=<0>.

- Dropped Tegra124 because there is no enough information about how to
  properly calculate required EMC clock rate for it and I don't have
  hardware for testing. Somebody else will have to work on it.

- Moved interconnect providers code into drivers/memory/tegra/*.

- Added "Create tegra20-devfreq device" patch because interconnect
  is not very usable without the devfreq memory auto-scaling since
  memory freq will be fixed to the display's requirement.

Dmitry Osipenko (36):
  clk: Export clk_hw_reparent()
  clk: tegra: Remove Memory Controller lock
  clk: tegra: Export Tegra20 EMC kernel symbols
  memory: tegra20-emc: Make driver modular
  memory: tegra30-emc: Make driver modular
  memory: tegra124-emc: Make driver modular
  memory: tegra124-emc: Use devm_platform_ioremap_resource
  soc/teg

[PATCH RESEND v10 1/4] drm/tegra: output: Don't leak OF node on error

2020-08-17 Thread Dmitry Osipenko
The OF node should be put before returning error in tegra_output_probe(),
otherwise node's refcount will be leaked.

Reviewed-by: Laurent Pinchart 
Reviewed-by: Sam Ravnborg 
Signed-off-by: Dmitry Osipenko 
---
 drivers/gpu/drm/tegra/output.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
index e36e5e7c2f69..a6a711d54e88 100644
--- a/drivers/gpu/drm/tegra/output.c
+++ b/drivers/gpu/drm/tegra/output.c
@@ -102,10 +102,10 @@ int tegra_output_probe(struct tegra_output *output)
panel = of_parse_phandle(output->of_node, "nvidia,panel", 0);
if (panel) {
output->panel = of_drm_find_panel(panel);
+   of_node_put(panel);
+
if (IS_ERR(output->panel))
return PTR_ERR(output->panel);
-
-   of_node_put(panel);
}
 
output->edid = of_get_property(output->of_node, "nvidia,edid", );
@@ -113,13 +113,12 @@ int tegra_output_probe(struct tegra_output *output)
ddc = of_parse_phandle(output->of_node, "nvidia,ddc-i2c-bus", 0);
if (ddc) {
output->ddc = of_find_i2c_adapter_by_node(ddc);
+   of_node_put(ddc);
+
if (!output->ddc) {
err = -EPROBE_DEFER;
-   of_node_put(ddc);
return err;
}
-
-   of_node_put(ddc);
}
 
output->hpd_gpio = devm_gpiod_get_from_of_node(output->dev,
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 23/36] dt-bindings: memory: tegra20: Add memory client IDs

2020-08-17 Thread Dmitry Osipenko
Each memory client have a unique hardware ID, this patch adds these IDs.

Acked-by: Rob Herring 
Signed-off-by: Dmitry Osipenko 
---
 include/dt-bindings/memory/tegra20-mc.h | 53 +
 1 file changed, 53 insertions(+)

diff --git a/include/dt-bindings/memory/tegra20-mc.h 
b/include/dt-bindings/memory/tegra20-mc.h
index 35e131eee198..6f8829508ad0 100644
--- a/include/dt-bindings/memory/tegra20-mc.h
+++ b/include/dt-bindings/memory/tegra20-mc.h
@@ -18,4 +18,57 @@
 #define TEGRA20_MC_RESET_VDE   13
 #define TEGRA20_MC_RESET_VI14
 
+#define TEGRA20_MC_DISPLAY0A   0
+#define TEGRA20_MC_DISPLAY0AB  1
+#define TEGRA20_MC_DISPLAY0B   2
+#define TEGRA20_MC_DISPLAY0BB  3
+#define TEGRA20_MC_DISPLAY0C   4
+#define TEGRA20_MC_DISPLAY0CB  5
+#define TEGRA20_MC_DISPLAY1B   6
+#define TEGRA20_MC_DISPLAY1BB  7
+#define TEGRA20_MC_EPPUP   8
+#define TEGRA20_MC_G2PR9
+#define TEGRA20_MC_G2SR10
+#define TEGRA20_MC_MPEUNIFBR   11
+#define TEGRA20_MC_VIRUV   12
+#define TEGRA20_MC_AVPCARM7R   13
+#define TEGRA20_MC_DISPLAYHC   14
+#define TEGRA20_MC_DISPLAYHCB  15
+#define TEGRA20_MC_FDCDRD  16
+#define TEGRA20_MC_G2DR17
+#define TEGRA20_MC_HOST1XDMAR  18
+#define TEGRA20_MC_HOST1XR 19
+#define TEGRA20_MC_IDXSRD  20
+#define TEGRA20_MC_MPCORER 21
+#define TEGRA20_MC_MPE_IPRED   22
+#define TEGRA20_MC_MPEAMEMRD   23
+#define TEGRA20_MC_MPECSRD 24
+#define TEGRA20_MC_PPCSAHBDMAR 25
+#define TEGRA20_MC_PPCSAHBSLVR 26
+#define TEGRA20_MC_TEXSRD  27
+#define TEGRA20_MC_VDEBSEVR28
+#define TEGRA20_MC_VDEMBER 29
+#define TEGRA20_MC_VDEMCER 30
+#define TEGRA20_MC_VDETPER 31
+#define TEGRA20_MC_EPPU32
+#define TEGRA20_MC_EPPV33
+#define TEGRA20_MC_EPPY34
+#define TEGRA20_MC_MPEUNIFBW   35
+#define TEGRA20_MC_VIWSB   36
+#define TEGRA20_MC_VIWU37
+#define TEGRA20_MC_VIWV38
+#define TEGRA20_MC_VIWY39
+#define TEGRA20_MC_G2DW40
+#define TEGRA20_MC_AVPCARM7W   41
+#define TEGRA20_MC_FDCDWR  42
+#define TEGRA20_MC_HOST1XW 43
+#define TEGRA20_MC_ISPW44
+#define TEGRA20_MC_MPCOREW 45
+#define TEGRA20_MC_MPECSWR 46
+#define TEGRA20_MC_PPCSAHBDMAW 47
+#define TEGRA20_MC_PPCSAHBSLVW 48
+#define TEGRA20_MC_VDEBSEVW49
+#define TEGRA20_MC_VDEMBEW 50
+#define TEGRA20_MC_VDETPMW 51
+
 #endif
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 36/36] drm/tegra: dc: Extend debug stats with total number of events

2020-08-17 Thread Dmitry Osipenko
It's useful to know the total number of underflow events and currently
the debug stats are getting reset each time CRTC is being disabled. Let's
account the overall number of events that doesn't get reset.

Signed-off-by: Dmitry Osipenko 
---
 drivers/gpu/drm/tegra/dc.c | 10 ++
 drivers/gpu/drm/tegra/dc.h |  5 +
 2 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 4b062408467e..0692b9f0ec29 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -1644,6 +1644,11 @@ static int tegra_dc_show_stats(struct seq_file *s, void 
*data)
seq_printf(s, "underflow: %lu\n", dc->stats.underflow);
seq_printf(s, "overflow: %lu\n", dc->stats.overflow);
 
+   seq_printf(s, "frames total: %lu\n", dc->stats.frames_total);
+   seq_printf(s, "vblank total: %lu\n", dc->stats.vblank_total);
+   seq_printf(s, "underflow total: %lu\n", dc->stats.underflow_total);
+   seq_printf(s, "overflow total: %lu\n", dc->stats.overflow_total);
+
return 0;
 }
 
@@ -2206,6 +2211,7 @@ static irqreturn_t tegra_dc_irq(int irq, void *data)
/*
dev_dbg(dc->dev, "%s(): frame end\n", __func__);
*/
+   dc->stats.frames_total++;
dc->stats.frames++;
}
 
@@ -2214,6 +2220,7 @@ static irqreturn_t tegra_dc_irq(int irq, void *data)
dev_dbg(dc->dev, "%s(): vertical blank\n", __func__);
*/
drm_crtc_handle_vblank(>base);
+   dc->stats.vblank_total++;
dc->stats.vblank++;
}
 
@@ -2221,6 +2228,7 @@ static irqreturn_t tegra_dc_irq(int irq, void *data)
/*
dev_dbg(dc->dev, "%s(): underflow\n", __func__);
*/
+   dc->stats.underflow_total++;
dc->stats.underflow++;
}
 
@@ -2228,11 +2236,13 @@ static irqreturn_t tegra_dc_irq(int irq, void *data)
/*
dev_dbg(dc->dev, "%s(): overflow\n", __func__);
*/
+   dc->stats.overflow_total++;
dc->stats.overflow++;
}
 
if (status & HEAD_UF_INT) {
dev_dbg_ratelimited(dc->dev, "%s(): head underflow\n", 
__func__);
+   dc->stats.underflow_total++;
dc->stats.underflow++;
}
 
diff --git a/drivers/gpu/drm/tegra/dc.h b/drivers/gpu/drm/tegra/dc.h
index b70e2033..41ca33abb84c 100644
--- a/drivers/gpu/drm/tegra/dc.h
+++ b/drivers/gpu/drm/tegra/dc.h
@@ -41,6 +41,11 @@ struct tegra_dc_stats {
unsigned long vblank;
unsigned long underflow;
unsigned long overflow;
+
+   unsigned long frames_total;
+   unsigned long vblank_total;
+   unsigned long underflow_total;
+   unsigned long overflow_total;
 };
 
 struct tegra_windowgroup_soc {
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 20/36] dt-bindings: memory: tegra30: mc: Document new interconnect property

2020-08-17 Thread Dmitry Osipenko
Memory controller is interconnected with memory clients and with the
external memory controller. Document new interconnect property which
turns memory controller into interconnect provider.

Acked-by: Rob Herring 
Signed-off-by: Dmitry Osipenko 
---
 .../bindings/memory-controllers/nvidia,tegra30-mc.yaml   | 5 +
 1 file changed, 5 insertions(+)

diff --git 
a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml 
b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml
index 84fd57bcf0dc..5436e6d420bc 100644
--- 
a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml
+++ 
b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml
@@ -57,6 +57,9 @@ properties:
   "#iommu-cells":
 const: 1
 
+  "#interconnect-cells":
+const: 1
+
 patternProperties:
   "^emc-timings-[0-9]+$":
 type: object
@@ -120,6 +123,7 @@ required:
   - clock-names
   - "#reset-cells"
   - "#iommu-cells"
+  - "#interconnect-cells"
 
 additionalProperties: false
 
@@ -135,6 +139,7 @@ examples:
 
 #iommu-cells = <1>;
 #reset-cells = <1>;
+#interconnect-cells = <1>;
 
 emc-timings-1 {
 nvidia,ram-code = <1>;
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v5 13/36] PM / devfreq: tegra30: Use MC timings for building OPP table

2020-08-17 Thread Dmitry Osipenko
14.08.2020 05:02, Chanwoo Choi пишет:
> Hi Dmitry,
> 
> I add the minor comment. Except of some comments, it looks good to me.

Hello, Chanwoo! Thank you for the review!

...
>> +struct tegra_devfreq_soc_data {
>> +const char *mc_compatible;
>> +};
>> +
>> +static const struct tegra_devfreq_soc_data tegra30_soc = {
>> +.mc_compatible = "nvidia,tegra30-mc",
>> +};
>> +
>> +static const struct tegra_devfreq_soc_data tegra124_soc = {
>> +.mc_compatible = "nvidia,tegra124-mc",
>> +};
.
>> +soc_data = of_device_get_match_data(>dev);
> 
> I think that better to check whether 'soc_data' is not NULL.

It's a quite common misconception among kernel developers that
of_device_get_match_data() may "accidentally" return NULL, but it
couldn't if every driver's of_match[] entry has a non-NULL .data field
and because the OF-matching already happened at the driver's probe-time
[1], which is the case of this driver.

[1] https://elixir.bootlin.com/linux/v5.8/source/drivers/of/device.c#L189

Hence the NULL-checking is unnecessary.

When I first encountered the of_device_get_match_data(), I was also
thinking that adding the NULL-checks is a good idea, but later on
somebody pointed out to me (maybe Thierry) that it's unnecessary to do.

>> +
>> +mc = tegra_get_memory_controller(soc_data->mc_compatible);
>> +if (IS_ERR(mc))
>> +return PTR_ERR(mc);
> 
> You better to add error log.

In practice we should get only -EPROBE_DEFER here ever. I'll consider
adding the message in the next revision, at least just for consistency.

Thanks!

...
>>  static const struct of_device_id tegra_devfreq_of_match[] = {
>> -{ .compatible = "nvidia,tegra30-actmon" },
>> -{ .compatible = "nvidia,tegra124-actmon" },
>> +{ .compatible = "nvidia,tegra30-actmon",  .data = _soc, },
>> +{ .compatible = "nvidia,tegra124-actmon", .data = _soc, },
>>  { },
>>  };


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH RESEND v10 3/4] drm/tegra: output: rgb: Support LVDS encoder bridge

2020-08-17 Thread Dmitry Osipenko
Newer Tegra device-trees will specify a video output graph, which involves
LVDS encoder bridge. This patch adds support for the LVDS encoder bridge
to the RGB output, allowing us to model the display hardware properly.

Reviewed-by: Laurent Pinchart 
Acked-by: Sam Ravnborg 
Signed-off-by: Dmitry Osipenko 
---
 drivers/gpu/drm/tegra/rgb.c | 58 +++--
 1 file changed, 49 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/tegra/rgb.c b/drivers/gpu/drm/tegra/rgb.c
index 0562a7eb793f..9a7024ec96bc 100644
--- a/drivers/gpu/drm/tegra/rgb.c
+++ b/drivers/gpu/drm/tegra/rgb.c
@@ -7,6 +7,7 @@
 #include 
 
 #include 
+#include 
 #include 
 #include 
 
@@ -267,24 +268,63 @@ int tegra_dc_rgb_remove(struct tegra_dc *dc)
 int tegra_dc_rgb_init(struct drm_device *drm, struct tegra_dc *dc)
 {
struct tegra_output *output = dc->rgb;
+   struct drm_connector *connector;
int err;
 
if (!dc->rgb)
return -ENODEV;
 
-   drm_connector_init(drm, >connector, _rgb_connector_funcs,
-  DRM_MODE_CONNECTOR_LVDS);
-   drm_connector_helper_add(>connector,
-_rgb_connector_helper_funcs);
-   output->connector.dpms = DRM_MODE_DPMS_OFF;
-
drm_simple_encoder_init(drm, >encoder, DRM_MODE_ENCODER_LVDS);
drm_encoder_helper_add(>encoder,
   _rgb_encoder_helper_funcs);
 
-   drm_connector_attach_encoder(>connector,
- >encoder);
-   drm_connector_register(>connector);
+   /*
+* Tegra devices that have LVDS panel utilize LVDS encoder bridge
+* for converting up to 28 LCD LVTTL lanes into 5/4 LVDS lanes that
+* go to display panel's receiver.
+*
+* Encoder usually have a power-down control which needs to be enabled
+* in order to transmit data to the panel.  Historically devices that
+* use an older device-tree version didn't model the bridge, assuming
+* that encoder is turned ON by default, while today's DRM allows us
+* to model LVDS encoder properly.
+*
+* Newer device-trees utilize LVDS encoder bridge, which provides
+* us with a connector and handles the display panel.
+*
+* For older device-trees we fall back to our own connector and use
+* nvidia,panel phandle.
+*/
+   if (output->bridge) {
+   err = drm_bridge_attach(>encoder, output->bridge,
+   NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
+   if (err) {
+   dev_err(output->dev, "failed to attach bridge: %d\n",
+   err);
+   return err;
+   }
+
+   connector = drm_bridge_connector_init(drm, >encoder);
+   if (IS_ERR(connector)) {
+   dev_err(output->dev,
+   "failed to initialize bridge connector: %pe\n",
+   connector);
+   return PTR_ERR(connector);
+   }
+
+   drm_connector_attach_encoder(connector, >encoder);
+   } else {
+   drm_connector_init(drm, >connector,
+  _rgb_connector_funcs,
+  DRM_MODE_CONNECTOR_LVDS);
+   drm_connector_helper_add(>connector,
+_rgb_connector_helper_funcs);
+   output->connector.dpms = DRM_MODE_DPMS_OFF;
+
+   drm_connector_attach_encoder(>connector,
+>encoder);
+   drm_connector_register(>connector);
+   }
 
err = tegra_output_init(drm, output);
if (err < 0) {
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH RESEND v12 2/4] drm/panel: Read panel orientation for BOE TV101WUM-NL6

2020-08-17 Thread Dmitry Osipenko
From: Derek Basehore 

This reads the DT setting for the panel rotation to set the panel
orientation in the get_modes callback.

Reviewed-by: Dmitry Osipenko 
Signed-off-by: Derek Basehore 
Signed-off-by: Dmitry Osipenko 
---
 drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c 
b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
index e320aa30b9ae..6824363fcff1 100644
--- a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
+++ b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
@@ -11,6 +11,7 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -43,6 +44,7 @@ struct boe_panel {
 
const struct panel_desc *desc;
 
+   enum drm_panel_orientation orientation;
struct regulator *pp1800;
struct regulator *avee;
struct regulator *avdd;
@@ -740,6 +742,7 @@ static int boe_panel_get_modes(struct drm_panel *panel,
connector->display_info.width_mm = boe->desc->size.width_mm;
connector->display_info.height_mm = boe->desc->size.height_mm;
connector->display_info.bpc = boe->desc->bpc;
+   drm_connector_set_panel_orientation(connector, boe->orientation);
 
return 1;
 }
@@ -779,6 +782,9 @@ static int boe_panel_add(struct boe_panel *boe)
 
drm_panel_init(>base, dev, _panel_funcs,
   DRM_MODE_CONNECTOR_DSI);
+   err = of_drm_get_panel_orientation(dev->of_node, >orientation);
+   if (err < 0)
+   return err;
 
err = drm_panel_of_backlight(>base);
if (err)
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH RESEND v10 2/4] drm/tegra: output: Support DRM bridges

2020-08-17 Thread Dmitry Osipenko
Newer Tegra device-trees will specify a video output graph which involves
a bridge. This patch adds initial support for the DRM bridges to the Tegra
DRM output.

Acked-by: Sam Ravnborg 
Signed-off-by: Dmitry Osipenko 
---
 drivers/gpu/drm/tegra/drm.h|  2 ++
 drivers/gpu/drm/tegra/output.c | 12 
 2 files changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index b25443255be6..f38de08e0c95 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -12,6 +12,7 @@
 #include 
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -116,6 +117,7 @@ struct tegra_output {
struct device_node *of_node;
struct device *dev;
 
+   struct drm_bridge *bridge;
struct drm_panel *panel;
struct i2c_adapter *ddc;
const struct edid *edid;
diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
index a6a711d54e88..ccd1421f1b24 100644
--- a/drivers/gpu/drm/tegra/output.c
+++ b/drivers/gpu/drm/tegra/output.c
@@ -5,6 +5,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 
@@ -99,8 +100,19 @@ int tegra_output_probe(struct tegra_output *output)
if (!output->of_node)
output->of_node = output->dev->of_node;
 
+   err = drm_of_find_panel_or_bridge(output->of_node, -1, -1,
+ >panel, >bridge);
+   if (err && err != -ENODEV)
+   return err;
+
panel = of_parse_phandle(output->of_node, "nvidia,panel", 0);
if (panel) {
+   /*
+* Don't mix nvidia,panel phandle with the graph in a
+* device-tree.
+*/
+   WARN_ON(output->panel || output->bridge);
+
output->panel = of_drm_find_panel(panel);
of_node_put(panel);
 
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 14/36] PM / devfreq: tegra20: Add error messages to tegra_devfreq_target()

2020-08-17 Thread Dmitry Osipenko
It's useful to now when something goes wrong instead of failing silently,
so let's add error messages to tegra_devfreq_target() to prevent situation
where it fails silently.

Acked-by: Chanwoo Choi 
Signed-off-by: Dmitry Osipenko 
---
 drivers/devfreq/tegra20-devfreq.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/devfreq/tegra20-devfreq.c 
b/drivers/devfreq/tegra20-devfreq.c
index a985f24098f5..bc43284996de 100644
--- a/drivers/devfreq/tegra20-devfreq.c
+++ b/drivers/devfreq/tegra20-devfreq.c
@@ -44,19 +44,25 @@ static int tegra_devfreq_target(struct device *dev, 
unsigned long *freq,
int err;
 
opp = devfreq_recommended_opp(dev, freq, flags);
-   if (IS_ERR(opp))
+   if (IS_ERR(opp)) {
+   dev_err(dev, "failed to find opp for %lu Hz\n", *freq);
return PTR_ERR(opp);
+   }
 
rate = dev_pm_opp_get_freq(opp);
dev_pm_opp_put(opp);
 
err = clk_set_min_rate(tegra->emc_clock, rate);
-   if (err)
+   if (err) {
+   dev_err(dev, "failed to set min rate: %d\n", err);
return err;
+   }
 
err = clk_set_rate(tegra->emc_clock, 0);
-   if (err)
+   if (err) {
+   dev_err(dev, "failed to set rate: %d\n", err);
goto restore_min_rate;
+   }
 
return 0;
 
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH RESEND v12 3/4] drm/panel: lvds: Read panel orientation

2020-08-17 Thread Dmitry Osipenko
The panel orientation needs to parsed from a device-tree and assigned to
the panel's connector in order to make orientation property available to
userspace. That's what this patch does for the generic LVDS panel.

Signed-off-by: Dmitry Osipenko 
---
 drivers/gpu/drm/panel/panel-lvds.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-lvds.c 
b/drivers/gpu/drm/panel/panel-lvds.c
index 5ce3f4a2b7a1..f62227059090 100644
--- a/drivers/gpu/drm/panel/panel-lvds.c
+++ b/drivers/gpu/drm/panel/panel-lvds.c
@@ -37,6 +37,8 @@ struct panel_lvds {
 
struct gpio_desc *enable_gpio;
struct gpio_desc *reset_gpio;
+
+   enum drm_panel_orientation orientation;
 };
 
 static inline struct panel_lvds *to_panel_lvds(struct drm_panel *panel)
@@ -99,6 +101,7 @@ static int panel_lvds_get_modes(struct drm_panel *panel,
connector->display_info.bus_flags = lvds->data_mirror
  ? DRM_BUS_FLAG_DATA_LSB_TO_MSB
  : DRM_BUS_FLAG_DATA_MSB_TO_LSB;
+   drm_connector_set_panel_orientation(connector, lvds->orientation);
 
return 1;
 }
@@ -116,6 +119,13 @@ static int panel_lvds_parse_dt(struct panel_lvds *lvds)
const char *mapping;
int ret;
 
+   ret = of_drm_get_panel_orientation(np, >orientation);
+   if (ret < 0) {
+   dev_err(lvds->dev, "%pOF: problems parsing rotation (%d)\n",
+   np, ret);
+   return ret;
+   }
+
ret = of_get_display_timing(np, "panel-timing", );
if (ret < 0) {
dev_err(lvds->dev, "%pOF: problems parsing panel-timing (%d)\n",
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 08/36] soc/tegra: fuse: Export tegra_read_ram_code()

2020-08-17 Thread Dmitry Osipenko
The tegra_read_ram_code() is used by EMC drivers and we're going to make
these driver modular, hence this function needs to be exported.

Signed-off-by: Dmitry Osipenko 
---
 drivers/soc/tegra/fuse/tegra-apbmisc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/soc/tegra/fuse/tegra-apbmisc.c 
b/drivers/soc/tegra/fuse/tegra-apbmisc.c
index 8e416ad91ee2..151eccb6069d 100644
--- a/drivers/soc/tegra/fuse/tegra-apbmisc.c
+++ b/drivers/soc/tegra/fuse/tegra-apbmisc.c
@@ -3,6 +3,7 @@
  * Copyright (c) 2014, NVIDIA CORPORATION.  All rights reserved.
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -65,6 +66,7 @@ u32 tegra_read_ram_code(void)
 
return straps >> PMC_STRAPPING_OPT_A_RAM_CODE_SHIFT;
 }
+EXPORT_SYMBOL_GPL(tegra_read_ram_code);
 
 static const struct of_device_id apbmisc_match[] __initconst = {
{ .compatible = "nvidia,tegra20-apbmisc", },
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 25/36] ARM: tegra: Add interconnect properties to Tegra20 device-tree

2020-08-17 Thread Dmitry Osipenko
Add interconnect properties to the memory controller, external memory
controller and the display controller nodes in order to describe hardware
interconnection.

Signed-off-by: Dmitry Osipenko 
---
 arch/arm/boot/dts/tegra20.dtsi | 22 +-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index 72a4211a618f..629ad101c43b 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -111,6 +111,15 @@ dc@5420 {
 
nvidia,head = <0>;
 
+   interconnects = < TEGRA20_MC_DISPLAY0A >,
+   < TEGRA20_MC_DISPLAY0B >,
+   < TEGRA20_MC_DISPLAY0C >,
+   < TEGRA20_MC_DISPLAY1B >;
+   interconnect-names = "wina",
+"winb",
+"winc",
+"cursor";
+
rgb {
status = "disabled";
};
@@ -128,6 +137,15 @@ dc@5424 {
 
nvidia,head = <1>;
 
+   interconnects = < TEGRA20_MC_DISPLAY0AB >,
+   < TEGRA20_MC_DISPLAY0BB >,
+   < TEGRA20_MC_DISPLAY0CB >,
+   < TEGRA20_MC_DISPLAY1BB >;
+   interconnect-names = "wina",
+"winb",
+"winc",
+"cursor";
+
rgb {
status = "disabled";
};
@@ -630,15 +648,17 @@ mc: memory-controller@7000f000 {
interrupts = ;
#reset-cells = <1>;
#iommu-cells = <0>;
+   #interconnect-cells = <1>;
};
 
-   memory-controller@7000f400 {
+   emc: memory-controller@7000f400 {
compatible = "nvidia,tegra20-emc";
reg = <0x7000f400 0x200>;
interrupts = ;
clocks = <_car TEGRA20_CLK_EMC>;
#address-cells = <1>;
#size-cells = <0>;
+   #interconnect-cells = <0>;
};
 
fuse@7000f800 {
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 01/36] clk: Export clk_hw_reparent()

2020-08-17 Thread Dmitry Osipenko
We're going to turn Tegra124 Memory Controller driver into a loadable
kernel module. The driver uses clk_hw_reparent(), which isn't an exported
kernel symbol. Let's export that function in order to allow modularization
of the Tegra driver.

Signed-off-by: Dmitry Osipenko 
---
 drivers/clk/clk.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 0a9261a099bd..2cbaede80fe5 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2448,6 +2448,7 @@ void clk_hw_reparent(struct clk_hw *hw, struct clk_hw 
*new_parent)
 
clk_core_reparent(hw->core, !new_parent ? NULL : new_parent->core);
 }
+EXPORT_SYMBOL_GPL(clk_hw_reparent);
 
 /**
  * clk_has_parent - check if a clock is a possible parent for another
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v11 0/4] Panel rotation patches

2020-07-20 Thread Dmitry Osipenko
18.06.2020 02:18, Dmitry Osipenko пишет:
> Hello!
> 
> This series adds support for display panel's DT rotation property. It's a
> continuation of the work that was initially started by Derek Basehore for
> the panel driver that is used by some Mediatek device [1]. I picked up the
> Derek's patches and added my t-b and r-b tags to them, I also added
> rotation support to the panel-lvds and panel-simple drivers.
> 
> We need the rotation support for the Nexus 7 tablet device which is pending
> to become supported by upstream kernel, the device has display panel mounted
> upside-down and it uses panel-lvds [2].
> 
> [1] https://lkml.org/lkml/2020/3/5/1119
> [2] 
> https://patchwork.ozlabs.org/project/linux-tegra/patch/20200607154327.18589-3-dig...@gmail.com/
> 
> Changelog:
> 
> v11: - This series is factored out from this patchset [3] because these
>patches do not have hard dependency on the Tegra DRM patches and
>it should be nicer to review and apply the properly grouped patches.
> 
>  - Initially [3] only touched the panel-lvds driver and Emil Velikov
>suggested that it will be better to support more panels in the review
>comments to [3]. So I included the Derek's patch for the BOE panel
>and added rotation support to the panel-simple driver. I tested that
>panel-lvds and panel-simple work properly with the rotated panel using
>the Opentegra Xorg driver [4] and Wayland Weston [5].
> 
>  - The panel-lvds driver now prints a error message if rotation property
>fails to be parsed.
> 
> [3] https://lore.kernel.org/lkml/20200614200121.14147-1-dig...@gmail.com/
> [4] 
> https://github.com/grate-driver/xf86-video-opentegra/commit/28eb20a3959bbe5bc3a3b67e55977093fd5114ca
> [5] https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/315
> 
> Derek Basehore (2):
>   drm/panel: Add helper for reading DT rotation
>   drm/panel: Read panel orientation for BOE TV101WUM-NL6
> 
> Dmitry Osipenko (2):
>   drm/panel: lvds: Read panel orientation
>   drm/panel-simple: Read panel orientation
> 
>  drivers/gpu/drm/drm_panel.c   | 43 +++
>  .../gpu/drm/panel/panel-boe-tv101wum-nl6.c|  6 +++
>  drivers/gpu/drm/panel/panel-lvds.c| 10 +
>  drivers/gpu/drm/panel/panel-simple.c  | 11 +
>  include/drm/drm_panel.h   |  9 
>  5 files changed, 79 insertions(+)
> 

Hi! Does anyone have any comments to this patchset? Will be nice if it
could get into 5.9 :) Thanks in advance!
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC] Host1x/TegraDRM UAPI (sync points)

2020-07-10 Thread Dmitry Osipenko
08.07.2020 13:06, Mikko Perttunen пишет:
> On 7/7/20 2:06 PM, Dmitry Osipenko wrote:
>> 02.07.2020 15:10, Mikko Perttunen пишет:
>>> Ok, so we would have two kinds of syncpoints for the job; one
>>> for kernel job tracking; and one that userspace can
>>> manipulate as it wants to.
>>>
>>> Could we handle the job tracking syncpoint completely inside the kernel,
>>> i.e. allocate it in kernel during job submission, and add an increment
>>> for it at the end of the job (with condition OP_DONE)? For MLOCKing, the
>>> kernel already needs to insert a SYNCPT_INCR(OP_DONE) + WAIT +
>>> MLOCK_RELEASE sequence at the end of each job.
>>
>> If sync point is allocated within kernel, then we'll need to always
>> patch all job's sync point increments with the ID of the allocated sync
>> point, regardless of whether firewall enabled or not.
> 
> The idea was that the job tracking increment would also be added to the
> pushbuffer in the kernel, so gathers would only have increments for the
> "user syncpoints", if any. I think this should work for THI-based
> engines (e.g. VIC), you probably have better information about
> GR2D/GR3D. On newer Tegras we could use CHANNEL/APPID protection to
> prevent the gathers from incrementing these job tracking syncpoints.

Could you please clarify what is THI?

>> Secondly, I'm now recalling that only one sync point could be assigned
>> to a channel at a time on newer Tegras which support sync point
>> protection. So it sounds like we don't really have variants other than
>> to allocate one sync point per channel for the jobs usage if we want to
>> be able to push multiple jobs into channel's pushbuffer, correct?
>>
> 
> The other way around; each syncpoint can be assigned to one channel. One
> channel may have multiple syncpoints.

Okay! So we actually could use a single sync point per-job for user
increments + job tracking, correct?

>> ...
>>>> Hmm, we actually should be able to have a one sync point per-channel
>>>> for
>>>> the job submission, similarly to what the current driver does!
>>>>
>>>> I'm keep forgetting about the waitbase existence!
>>>
>>> Tegra194 doesn't have waitbases, but if we are resubmitting all the jobs
>>> anyway, can't we just recalculate wait thresholds at that time?
>>
>> Yes, thresholds could be recalculated + job should be re-formed at the
>> push-time then.
>>
>> It also means that jobs always should be formed only at the push-time if
>> wait-command is utilized by cmdstream since the waits always need to be
>> patched because we won't know the thresholds until scheduler actually
>> runs the job.
> 
> Could we keep the job tracking syncpoints entirely within the kernel,
> and have all wait commands and other stuff that userspace does use the
> user syncpoints? Then kernel job tracking and userspace activity would
> be separate from each other.

I think this should work, but it's not clear to me what benefits this
brings to us if we could re-use the same user-allocated sync point for
both user increments + kernel job tracking.

Is the idea to protect from userspace incrementing sync point too much?
I.e. job could be treated as completed before CDMA is actually done with
it.

> Alternatively, if we know that jobs can only be removed from the middle
> of pushbuffers, and not added, we could replace any removed jobs with
> syncpoint increments in the pushbuffer and any thresholds would stay
> intact.

A bit unlikely that jobs could ever be removed from the middle of
hardware queue by the DRM scheduler.

Anyways, it should be nicer to shoot down everything and restart with a
clean slate when necessary, like it's already supposed to happen by the
scheduler.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC] Host1x/TegraDRM UAPI (sync points)

2020-07-08 Thread Dmitry Osipenko
02.07.2020 15:10, Mikko Perttunen пишет:
> Ok, so we would have two kinds of syncpoints for the job; one
> for kernel job tracking; and one that userspace can
> manipulate as it wants to.
> 
> Could we handle the job tracking syncpoint completely inside the kernel,
> i.e. allocate it in kernel during job submission, and add an increment
> for it at the end of the job (with condition OP_DONE)? For MLOCKing, the
> kernel already needs to insert a SYNCPT_INCR(OP_DONE) + WAIT +
> MLOCK_RELEASE sequence at the end of each job.

If sync point is allocated within kernel, then we'll need to always
patch all job's sync point increments with the ID of the allocated sync
point, regardless of whether firewall enabled or not.

Secondly, I'm now recalling that only one sync point could be assigned
to a channel at a time on newer Tegras which support sync point
protection. So it sounds like we don't really have variants other than
to allocate one sync point per channel for the jobs usage if we want to
be able to push multiple jobs into channel's pushbuffer, correct?

...
>> Hmm, we actually should be able to have a one sync point per-channel for
>> the job submission, similarly to what the current driver does!
>>
>> I'm keep forgetting about the waitbase existence!
> 
> Tegra194 doesn't have waitbases, but if we are resubmitting all the jobs
> anyway, can't we just recalculate wait thresholds at that time?

Yes, thresholds could be recalculated + job should be re-formed at the
push-time then.

It also means that jobs always should be formed only at the push-time if
wait-command is utilized by cmdstream since the waits always need to be
patched because we won't know the thresholds until scheduler actually
runs the job.

> Maybe a more detailed sequence list or diagram of what happens during
> submission and recovery would be useful.

The textual form + code is already good enough to me. A diagram could be
nice to have, although it may take a bit too much effort to create +
maintain it. But I don't mind at all if you'd want to make one :)

...
>>> * We should be able to keep the syncpoint refcounting based on fences.
>>
>> The fence doesn't need the sync point itself, it only needs to get a
>> signal when the threshold is reached or when sync point is ceased.
>>
>> Imagine:
>>
>>    - Process A creates sync point
>>    - Process A creates dma-fence from this sync point
>>    - Process A exports dma-fence to process B
>>    - Process A dies
>>
>> What should happen to process B?
>>
>>    - Should dma-fence of the process B get a error signal when process A
>> dies?
>>    - Should process B get stuck waiting endlessly for the dma-fence?
>>
>> This is one example of why I'm proposing that fence shouldn't be coupled
>> tightly to a sync point.
> 
> As a baseline, we should consider process B to get stuck endlessly
> (until a timeout of its choosing) for the fence. In this case it is
> avoidable, but if the ID/threshold pair is exported out of the fence and
> is waited for otherwise, it is unavoidable. I.e. once the ID/threshold
> are exported out of a fence, the waiter can only see the fence being
> signaled by the threshold being reached, not by the syncpoint getting
> freed.

This is correct. If sync point's FD is exported or once sync point is
resolved from a dma-fence, then sync point will stay alive until the
last reference to the sync point is dropped. I.e. if Process A dies
*after* process B started to wait on the sync point, then it will get stuck.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4 28/37] memory: tegra: Register as interconnect provider

2020-07-06 Thread Dmitry Osipenko
02.07.2020 15:36, Georgi Djakov пишет:
...
 +  mc->provider.data = data;
 +  mc->provider.xlate = of_icc_xlate_onecell;
 +  mc->provider.aggregate = tegra_mc_icc_aggregate;
 +
 +  err = icc_provider_add(>provider);
 +  if (err)
 +  goto err_msg;
>>>
>>> Nit: I am planning to re-organize some of the existing drivers to call
>>> icc_provider_add() after the topology is populated. Could you please move
>>> this after the nodes are created and linked.
>>
>> Are you planning to remove the provider's list-head initialization from
>> the icc_provider_add() [1] and move it to the individual provider
>> drivers, correct?
> 
> Yes, that would be the first step, but i need to post some patches first,
> so let's keep it as-is for now. Sorry for the confusion.

No problems, I'll keep an eye on the ICC core changes!
Please feel free to the CC me on the patches, I may give them a whirl.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4 12/37] PM / devfreq: tegra20: Use MC timings for building OPP table

2020-07-02 Thread Dmitry Osipenko
02.07.2020 08:30, Chanwoo Choi пишет:
> On 7/2/20 2:07 PM, Dmitry Osipenko wrote:
>> 02.07.2020 07:18, Chanwoo Choi пишет:
>>> Hi Dmitry,
>>>
>>> On 6/9/20 10:13 PM, Dmitry Osipenko wrote:
>>>> The clk_round_rate() won't be usable for building OPP table once
>>>> interconnect support will be added to the EMC driver because that CLK API
>>>> function limits the rounded rate based on the clk rate that is imposed by
>>>> active clk-users, and thus, the rounding won't work as expected if
>>>> interconnect will set the minimum EMC clock rate before devfreq driver is
>>>> loaded. The struct tegra_mc contains memory timings which could be used by
>>>> the devfreq driver for building up OPP table instead of rounding clock
>>>> rate, this patch implements this idea.
>>>>
>>>> Signed-off-by: Dmitry Osipenko 
>>>> ---
>>>>  drivers/devfreq/tegra20-devfreq.c | 18 +++---
>>>>  1 file changed, 11 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/devfreq/tegra20-devfreq.c 
>>>> b/drivers/devfreq/tegra20-devfreq.c
>>>> index 6469dc69c5e0..bf504ca4dea2 100644
>>>> --- a/drivers/devfreq/tegra20-devfreq.c
>>>> +++ b/drivers/devfreq/tegra20-devfreq.c
>>>> @@ -123,8 +123,7 @@ static int tegra_devfreq_probe(struct platform_device 
>>>> *pdev)
>>>>  {
>>>>struct tegra_devfreq *tegra;
>>>>struct tegra_mc *mc;
>>>> -  unsigned long max_rate;
>>>> -  unsigned long rate;
>>>> +  unsigned int i;
>>>>int err;
>>>>  
>>>>mc = tegra_get_memory_controller();
>>>> @@ -151,12 +150,17 @@ static int tegra_devfreq_probe(struct 
>>>> platform_device *pdev)
>>>>  
>>>>tegra->regs = mc->regs;
>>>>  
>>>> -  max_rate = clk_round_rate(tegra->emc_clock, ULONG_MAX);
>>>> -
>>>> -  for (rate = 0; rate <= max_rate; rate++) {
>>>> -  rate = clk_round_rate(tegra->emc_clock, rate);
>>>> +  if (!mc->num_timings) {
>>>
>>> Could you explain what is meaning of 'num_timing?
>>
>> The num_timings is the number of memory timings defined in a
>> device-tree. One timing configuration per memory clock rate.
> 
> OK. I understand.
> 
>>
>>> Also, why add the opp entry in case of mc->num_timings is zero?
>>
>> Timings may be not defined in some device-trees at all and in this case
>> memory always running on a fixed clock rate.
> 
> You mean that 'timings' information is optional?

Yes

Actually, looks like I missed to properly test this case where timings
are missing in DT and it shouldn't work with the current code. I'll fix
it in the next version.

>>
>> The devfreq driver won't be practically useful if mc->num_timings is
>> zero since memory frequency can't be changed, but anyways we'd want to
>> load the devfreq driver in order to prevent confusion about why it's not
>> loaded.
>>
>> For example, you may ask somebody to show contents of
>> /sys/class/devfreq/tegra20-devfreq/trans_stat and the person says to you
>> that this file doesn't exist, now you'll have to figure out what
>> happened to the devfreq driver.
> 
> I understand why add OPP entry point when timing is not defined on DT.
> But, actually, I think that you better to change 'timings' info is mandatory
> instead of optional. Because the devfreq driver is for DVFS
> and the driver supporting DVFS have to have the frequency information
> like OPP.
> 
> Or, 
> If you want to keep 'timing' is optional on DT,
> I recommend that you add one timing data to tegra mc driver
> when DT doesn't include the any timing information
> I think that is it more clear.

Okay, I'll move it into the MC driver in the next version.

Thank you for the review!
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4 27/37] interconnect: Relax requirement in of_icc_get_from_provider()

2020-07-02 Thread Dmitry Osipenko
01.07.2020 20:10, Georgi Djakov пишет:
> Hi Dmitry,
> 
> On 6/9/20 16:13, Dmitry Osipenko wrote:
>> From: Artur Świgoń 
>>
>> This patch relaxes the condition in of_icc_get_from_provider() so that it
>> is no longer required to set #interconnect-cells = <1> in the DT. In case
>> of the devfreq driver for exynos-bus, #interconnect-cells is always zero.
>>
>> Signed-off-by: Artur Świgoń 
>> [dig...@gmail.com: added cells_num checking for of_icc_xlate_onecell()]
>> Signed-off-by: Dmitry Osipenko 
> 
> I have already applied the original patch by Artur, so please make the 
> cells_num
> check a separate patch.

Okay, I'll update this patch! Thank you!
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4 11/37] PM / devfreq: tegra30: Silence deferred probe error

2020-07-02 Thread Dmitry Osipenko
02.07.2020 04:34, Chanwoo Choi пишет:
> On 7/2/20 10:20 AM, Dmitry Osipenko wrote:
>> 02.07.2020 03:59, Chanwoo Choi пишет:
>>> Hi,
>>>
>>> On 6/9/20 10:13 PM, Dmitry Osipenko wrote:
>>>> Tegra EMC driver was turned into a regular kernel driver, it also could
>>>> be compiled as a loadable kernel module now. Hence EMC clock isn't
>>>> guaranteed to be available and clk_get("emc") may return -EPROBE_DEFER and
>>>> there is no good reason to spam KMSG with a error about missing EMC clock
>>>> in this case, so let's silence the deferred probe error.
>>>>
>>>> Signed-off-by: Dmitry Osipenko 
>>>> ---
>>>>  drivers/devfreq/tegra30-devfreq.c | 9 ++---
>>>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/devfreq/tegra30-devfreq.c 
>>>> b/drivers/devfreq/tegra30-devfreq.c
>>>> index e94a27804c20..423dd35c95b3 100644
>>>> --- a/drivers/devfreq/tegra30-devfreq.c
>>>> +++ b/drivers/devfreq/tegra30-devfreq.c
>>>> @@ -801,9 +801,12 @@ static int tegra_devfreq_probe(struct platform_device 
>>>> *pdev)
>>>>}
>>>>  
>>>>tegra->emc_clock = devm_clk_get(>dev, "emc");
>>>> -  if (IS_ERR(tegra->emc_clock)) {
>>>> -  dev_err(>dev, "Failed to get emc clock\n");
>>>> -  return PTR_ERR(tegra->emc_clock);
>>>> +  err = PTR_ERR_OR_ZERO(tegra->emc_clock);
>>>> +  if (err) {
>>>> +  if (err != -EPROBE_DEFER)
>>>> +  dev_err(>dev, "Failed to get emc clock: %d\n",
>>>> +  err);
>>>> +  return err;
>>>>}
>>>>  
>>>>err = platform_get_irq(pdev, 0);
>>>>
>>>
>>> As I commented on patch10, I recommend that you add the Tegra EMC driver
>>> commit information into patch description and Looks good to me.
>>>
>>
>> Hello, Chanwoo!
>>
>> This patch11 and patch10 are depending on the patches 4/5 (the Tegra EMC
>> driver patches) of *this* series, hence there is no commit information.
>> I'm expecting that this whole series will go via tegra tree once all the
>> patches will be reviewed and collect all the necessary acks from you,
>> ICC and CLK subsystem maintainers.
>>
>> Please feel free to give yours ack to the patches 10/11 if they are good
>> to you :)
>>
>>
> 
> OK. Looks good to me
> Acked-by: Chanwoo Choi 
> 

Thank you! :)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4 12/37] PM / devfreq: tegra20: Use MC timings for building OPP table

2020-07-02 Thread Dmitry Osipenko
02.07.2020 07:18, Chanwoo Choi пишет:
> Hi Dmitry,
> 
> On 6/9/20 10:13 PM, Dmitry Osipenko wrote:
>> The clk_round_rate() won't be usable for building OPP table once
>> interconnect support will be added to the EMC driver because that CLK API
>> function limits the rounded rate based on the clk rate that is imposed by
>> active clk-users, and thus, the rounding won't work as expected if
>> interconnect will set the minimum EMC clock rate before devfreq driver is
>> loaded. The struct tegra_mc contains memory timings which could be used by
>> the devfreq driver for building up OPP table instead of rounding clock
>> rate, this patch implements this idea.
>>
>> Signed-off-by: Dmitry Osipenko 
>> ---
>>  drivers/devfreq/tegra20-devfreq.c | 18 +++---
>>  1 file changed, 11 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/devfreq/tegra20-devfreq.c 
>> b/drivers/devfreq/tegra20-devfreq.c
>> index 6469dc69c5e0..bf504ca4dea2 100644
>> --- a/drivers/devfreq/tegra20-devfreq.c
>> +++ b/drivers/devfreq/tegra20-devfreq.c
>> @@ -123,8 +123,7 @@ static int tegra_devfreq_probe(struct platform_device 
>> *pdev)
>>  {
>>  struct tegra_devfreq *tegra;
>>  struct tegra_mc *mc;
>> -unsigned long max_rate;
>> -unsigned long rate;
>> +unsigned int i;
>>  int err;
>>  
>>  mc = tegra_get_memory_controller();
>> @@ -151,12 +150,17 @@ static int tegra_devfreq_probe(struct platform_device 
>> *pdev)
>>  
>>  tegra->regs = mc->regs;
>>  
>> -max_rate = clk_round_rate(tegra->emc_clock, ULONG_MAX);
>> -
>> -for (rate = 0; rate <= max_rate; rate++) {
>> -rate = clk_round_rate(tegra->emc_clock, rate);
>> +if (!mc->num_timings) {
> 
> Could you explain what is meaning of 'num_timing?

The num_timings is the number of memory timings defined in a
device-tree. One timing configuration per memory clock rate.

> Also, why add the opp entry in case of mc->num_timings is zero?

Timings may be not defined in some device-trees at all and in this case
memory always running on a fixed clock rate.

The devfreq driver won't be practically useful if mc->num_timings is
zero since memory frequency can't be changed, but anyways we'd want to
load the devfreq driver in order to prevent confusion about why it's not
loaded.

For example, you may ask somebody to show contents of
/sys/class/devfreq/tegra20-devfreq/trans_stat and the person says to you
that this file doesn't exist, now you'll have to figure out what
happened to the devfreq driver.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4 11/37] PM / devfreq: tegra30: Silence deferred probe error

2020-07-02 Thread Dmitry Osipenko
02.07.2020 03:59, Chanwoo Choi пишет:
> Hi,
> 
> On 6/9/20 10:13 PM, Dmitry Osipenko wrote:
>> Tegra EMC driver was turned into a regular kernel driver, it also could
>> be compiled as a loadable kernel module now. Hence EMC clock isn't
>> guaranteed to be available and clk_get("emc") may return -EPROBE_DEFER and
>> there is no good reason to spam KMSG with a error about missing EMC clock
>> in this case, so let's silence the deferred probe error.
>>
>> Signed-off-by: Dmitry Osipenko 
>> ---
>>  drivers/devfreq/tegra30-devfreq.c | 9 ++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/devfreq/tegra30-devfreq.c 
>> b/drivers/devfreq/tegra30-devfreq.c
>> index e94a27804c20..423dd35c95b3 100644
>> --- a/drivers/devfreq/tegra30-devfreq.c
>> +++ b/drivers/devfreq/tegra30-devfreq.c
>> @@ -801,9 +801,12 @@ static int tegra_devfreq_probe(struct platform_device 
>> *pdev)
>>  }
>>  
>>  tegra->emc_clock = devm_clk_get(>dev, "emc");
>> -if (IS_ERR(tegra->emc_clock)) {
>> -dev_err(>dev, "Failed to get emc clock\n");
>> -return PTR_ERR(tegra->emc_clock);
>> +err = PTR_ERR_OR_ZERO(tegra->emc_clock);
>> +if (err) {
>> +if (err != -EPROBE_DEFER)
>> +dev_err(>dev, "Failed to get emc clock: %d\n",
>> +err);
>> +return err;
>>  }
>>  
>>  err = platform_get_irq(pdev, 0);
>>
> 
> As I commented on patch10, I recommend that you add the Tegra EMC driver
> commit information into patch description and Looks good to me.
> 

Hello, Chanwoo!

This patch11 and patch10 are depending on the patches 4/5 (the Tegra EMC
driver patches) of *this* series, hence there is no commit information.
I'm expecting that this whole series will go via tegra tree once all the
patches will be reviewed and collect all the necessary acks from you,
ICC and CLK subsystem maintainers.

Please feel free to give yours ack to the patches 10/11 if they are good
to you :)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v10 0/2] Silence missing-graph error for DRM bridges

2020-07-02 Thread Dmitry Osipenko
Hi!

This small series improves DRM bridges code by silencing a noisy error
coming from of-graph code for the device-trees that are missing a
display bridge graph.

  graph: no port node found in ...

One example where this error happens is an older bridge-less DTB used
in conjunction with a newer kernel which has a display controller driver
that supports DRM bridges.

Changelog:

v10:- Corrected doc-comment, unbroke the of_graph_get_next_endpoint() and
  improved commit's message in the "add of_graph_is_present()" patch.
  Thanks to Laurent Pinchart for spotting the problems!

v9: - These two patches are factored out from [1] in order to ease applying
  of the patches.

- The of_graph_presents() is renamed to of_graph_is_present() like it
  was requested by Rob Herring in the review comment to [1].

- Added Rob's r-b.

[1] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=184102

Dmitry Osipenko (2):
  of_graph: add of_graph_is_present()
  drm/of: Make drm_of_find_panel_or_bridge() to check graph's presence

 drivers/gpu/drm/drm_of.c |  9 +
 drivers/of/property.c| 23 +++
 include/linux/of_graph.h |  6 ++
 3 files changed, 38 insertions(+)

-- 
2.26.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4 12/37] PM / devfreq: tegra20: Use MC timings for building OPP table

2020-07-02 Thread Dmitry Osipenko
02.07.2020 08:43, Dmitry Osipenko пишет:
> 02.07.2020 08:30, Chanwoo Choi пишет:
>> On 7/2/20 2:07 PM, Dmitry Osipenko wrote:
>>> 02.07.2020 07:18, Chanwoo Choi пишет:
>>>> Hi Dmitry,
>>>>
>>>> On 6/9/20 10:13 PM, Dmitry Osipenko wrote:
>>>>> The clk_round_rate() won't be usable for building OPP table once
>>>>> interconnect support will be added to the EMC driver because that CLK API
>>>>> function limits the rounded rate based on the clk rate that is imposed by
>>>>> active clk-users, and thus, the rounding won't work as expected if
>>>>> interconnect will set the minimum EMC clock rate before devfreq driver is
>>>>> loaded. The struct tegra_mc contains memory timings which could be used by
>>>>> the devfreq driver for building up OPP table instead of rounding clock
>>>>> rate, this patch implements this idea.
>>>>>
>>>>> Signed-off-by: Dmitry Osipenko 
>>>>> ---
>>>>>  drivers/devfreq/tegra20-devfreq.c | 18 +++---
>>>>>  1 file changed, 11 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/drivers/devfreq/tegra20-devfreq.c 
>>>>> b/drivers/devfreq/tegra20-devfreq.c
>>>>> index 6469dc69c5e0..bf504ca4dea2 100644
>>>>> --- a/drivers/devfreq/tegra20-devfreq.c
>>>>> +++ b/drivers/devfreq/tegra20-devfreq.c
>>>>> @@ -123,8 +123,7 @@ static int tegra_devfreq_probe(struct platform_device 
>>>>> *pdev)
>>>>>  {
>>>>>   struct tegra_devfreq *tegra;
>>>>>   struct tegra_mc *mc;
>>>>> - unsigned long max_rate;
>>>>> - unsigned long rate;
>>>>> + unsigned int i;
>>>>>   int err;
>>>>>  
>>>>>   mc = tegra_get_memory_controller();
>>>>> @@ -151,12 +150,17 @@ static int tegra_devfreq_probe(struct 
>>>>> platform_device *pdev)
>>>>>  
>>>>>   tegra->regs = mc->regs;
>>>>>  
>>>>> - max_rate = clk_round_rate(tegra->emc_clock, ULONG_MAX);
>>>>> -
>>>>> - for (rate = 0; rate <= max_rate; rate++) {
>>>>> - rate = clk_round_rate(tegra->emc_clock, rate);
>>>>> + if (!mc->num_timings) {
>>>>
>>>> Could you explain what is meaning of 'num_timing?
>>>
>>> The num_timings is the number of memory timings defined in a
>>> device-tree. One timing configuration per memory clock rate.
>>
>> OK. I understand.
>>
>>>
>>>> Also, why add the opp entry in case of mc->num_timings is zero?
>>>
>>> Timings may be not defined in some device-trees at all and in this case
>>> memory always running on a fixed clock rate.
>>
>> You mean that 'timings' information is optional?
> 
> Yes
> 
> Actually, looks like I missed to properly test this case where timings
> are missing in DT and it shouldn't work with the current code. I'll fix
> it in the next version.
> 
>>>
>>> The devfreq driver won't be practically useful if mc->num_timings is
>>> zero since memory frequency can't be changed, but anyways we'd want to
>>> load the devfreq driver in order to prevent confusion about why it's not
>>> loaded.
>>>
>>> For example, you may ask somebody to show contents of
>>> /sys/class/devfreq/tegra20-devfreq/trans_stat and the person says to you
>>> that this file doesn't exist, now you'll have to figure out what
>>> happened to the devfreq driver.
>>
>> I understand why add OPP entry point when timing is not defined on DT.
>> But, actually, I think that you better to change 'timings' info is mandatory
>> instead of optional. Because the devfreq driver is for DVFS
>> and the driver supporting DVFS have to have the frequency information
>> like OPP.

That's what I initially did by bailing out from driver's probe if
timings info is missing, until ran into a situation described above :)

>> Or, 
>> If you want to keep 'timing' is optional on DT,
>> I recommend that you add one timing data to tegra mc driver
>> when DT doesn't include the any timing information
>> I think that is it more clear.
> 
> Okay, I'll move it into the MC driver in the next version.
> 
> Thank you for the review!
> 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v10 1/2] of_graph: add of_graph_is_present()

2020-07-02 Thread Dmitry Osipenko
In some cases it's very useful to silently check whether port node exists
at all in a device-tree before proceeding with parsing the graph. The DRM
bridges code is one example of such case where absence of a graph in a
device-tree is a legit condition.

This patch adds of_graph_is_present() which returns true if given
device-tree node contains OF graph port.

Reviewed-by: Rob Herring 
Signed-off-by: Dmitry Osipenko 
---
 drivers/of/property.c| 23 +++
 include/linux/of_graph.h |  6 ++
 2 files changed, 29 insertions(+)

diff --git a/drivers/of/property.c b/drivers/of/property.c
index 6a5760f0d6cd..fed7229d9d9f 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -29,6 +29,29 @@
 
 #include "of_private.h"
 
+/**
+ * of_graph_is_present() - check graph's presence
+ * @node: pointer to device_node containing graph port
+ *
+ * Return: True if @node has a port or ports (with a port) sub-node,
+ * false otherwise.
+ */
+bool of_graph_is_present(const struct device_node *node)
+{
+   struct device_node *ports, *port;
+
+   ports = of_get_child_by_name(node, "ports");
+   if (ports)
+   node = ports;
+
+   port = of_get_child_by_name(node, "port");
+   of_node_put(ports);
+   of_node_put(port);
+
+   return !!port;
+}
+EXPORT_SYMBOL(of_graph_is_present);
+
 /**
  * of_property_count_elems_of_size - Count the number of elements in a property
  *
diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h
index 01038a6aade0..4d7756087b6b 100644
--- a/include/linux/of_graph.h
+++ b/include/linux/of_graph.h
@@ -38,6 +38,7 @@ struct of_endpoint {
 child = of_graph_get_next_endpoint(parent, child))
 
 #ifdef CONFIG_OF
+bool of_graph_is_present(const struct device_node *node);
 int of_graph_parse_endpoint(const struct device_node *node,
struct of_endpoint *endpoint);
 int of_graph_get_endpoint_count(const struct device_node *np);
@@ -56,6 +57,11 @@ struct device_node *of_graph_get_remote_node(const struct 
device_node *node,
 u32 port, u32 endpoint);
 #else
 
+static inline bool of_graph_is_present(const struct device_node *node)
+{
+   return false;
+}
+
 static inline int of_graph_parse_endpoint(const struct device_node *node,
struct of_endpoint *endpoint)
 {
-- 
2.26.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v10 2/2] drm/of: Make drm_of_find_panel_or_bridge() to check graph's presence

2020-07-02 Thread Dmitry Osipenko
When graph isn't defined in a device-tree, the of_graph_get_remote_node()
prints a noisy error message, telling that port node is not found. This is
undesirable behaviour in our case because absence of a panel/bridge graph
is a valid case. Let's check the graph's presence in a device-tree before
proceeding with parsing of the graph.

Reviewed-by: Sam Ravnborg 
Signed-off-by: Dmitry Osipenko 
---
 drivers/gpu/drm/drm_of.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
index b50b44e76279..fdb05fbf72a0 100644
--- a/drivers/gpu/drm/drm_of.c
+++ b/drivers/gpu/drm/drm_of.c
@@ -246,6 +246,15 @@ int drm_of_find_panel_or_bridge(const struct device_node 
*np,
if (panel)
*panel = NULL;
 
+   /*
+* of_graph_get_remote_node() produces a noisy error message if port
+* node isn't found and the absence of the port is a legit case here,
+* so at first we silently check whether graph presents in the
+* device-tree node.
+*/
+   if (!of_graph_is_present(np))
+   return -ENODEV;
+
remote = of_graph_get_remote_node(np, port, endpoint);
if (!remote)
return -ENODEV;
-- 
2.26.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v10 0/2] Silence missing-graph error for DRM bridges

2020-07-02 Thread Dmitry Osipenko
01.07.2020 12:02, Sam Ravnborg пишет:
> Hi Dmitry
> On Wed, Jul 01, 2020 at 10:42:30AM +0300, Dmitry Osipenko wrote:
>> Hi!
>>
>> This small series improves DRM bridges code by silencing a noisy error
>> coming from of-graph code for the device-trees that are missing a
>> display bridge graph.
>>
>>   graph: no port node found in ...
>>
>> One example where this error happens is an older bridge-less DTB used
>> in conjunction with a newer kernel which has a display controller driver
>> that supports DRM bridges.
>>
>> Changelog:
>>
>> v10:- Corrected doc-comment, unbroke the of_graph_get_next_endpoint() and
>>   improved commit's message in the "add of_graph_is_present()" patch.
>>   Thanks to Laurent Pinchart for spotting the problems!
>>
>> v9: - These two patches are factored out from [1] in order to ease applying
>>   of the patches.
>>
>> - The of_graph_presents() is renamed to of_graph_is_present() like it
>>   was requested by Rob Herring in the review comment to [1].
>>
>> - Added Rob's r-b.
>>
>> [1] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=184102
>>
>> Dmitry Osipenko (2):
>>   of_graph: add of_graph_is_present()
>>   drm/of: Make drm_of_find_panel_or_bridge() to check graph's presence
> 
> Thanks for your patience with these - applied to drm-misc-next now.

Thanks to you and Laurent!
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4 28/37] memory: tegra: Register as interconnect provider

2020-07-02 Thread Dmitry Osipenko
01.07.2020 20:12, Georgi Djakov пишет:
> Hi Dmitry,
> 
> Thank you for updating the patches!

Hello, Georgi!

Thank you for the review!

> On 6/9/20 16:13, Dmitry Osipenko wrote:
>> Now memory controller is a memory interconnection provider. This allows us
>> to use interconnect API in order to change memory configuration.
>>
>> Signed-off-by: Dmitry Osipenko 
>> ---
>>  drivers/memory/tegra/Kconfig |   1 +
>>  drivers/memory/tegra/mc.c| 114 +++
>>  drivers/memory/tegra/mc.h|   8 +++
>>  include/soc/tegra/mc.h   |   3 +
>>  4 files changed, 126 insertions(+)
>>
>> diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
>> index 5bf75b316a2f..7055fdef2c32 100644
>> --- a/drivers/memory/tegra/Kconfig
>> +++ b/drivers/memory/tegra/Kconfig
>> @@ -3,6 +3,7 @@ config TEGRA_MC
>>  bool "NVIDIA Tegra Memory Controller support"
>>  default y
>>  depends on ARCH_TEGRA
>> +select INTERCONNECT
>>  help
>>This driver supports the Memory Controller (MC) hardware found on
>>NVIDIA Tegra SoCs.
>> diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
>> index 772aa021b5f6..7ef7ac9e103e 100644
>> --- a/drivers/memory/tegra/mc.c
>> +++ b/drivers/memory/tegra/mc.c
>> @@ -594,6 +594,118 @@ static __maybe_unused irqreturn_t tegra20_mc_irq(int 
>> irq, void *data)
>>  return IRQ_HANDLED;
>>  }
>>  
>> +static int tegra_mc_icc_set(struct icc_node *src, struct icc_node *dst)
>> +{
>> +return 0;
>> +}
>> +
>> +static int tegra_mc_icc_aggregate(struct icc_node *node,
>> +  u32 tag, u32 avg_bw, u32 peak_bw,
>> +  u32 *agg_avg, u32 *agg_peak)
>> +{
>> +*agg_avg = min((u64)avg_bw + (*agg_avg), (u64)U32_MAX);
>> +*agg_peak = max(*agg_peak, peak_bw);
>> +
>> +return 0;
>> +}
>> +
>> +/*
>> + * Memory Controller (MC) has few Memory Clients that are issuing memory
>> + * bandwidth allocation requests to the MC interconnect provider. The MC
>> + * provider aggregates the requests and then sends the aggregated request
>> + * up to the External Memory Controller (EMC) interconnect provider which
>> + * re-configures hardware interface to External Memory (EMEM) in accordance
>> + * to the required bandwidth. Each MC interconnect node represents an
>> + * individual Memory Client.
>> + *
>> + * Memory interconnect topology:
>> + *
>> + *   ++
>> + * ++||
>> + * | TEXSRD +--->+|
>> + * ++||
>> + *   ||+-++--+
>> + *...| MC +--->+ EMC +--->+ EMEM |
>> + *   ||+-++--+
>> + * ++||
>> + * | DISP.. +--->+|
>> + * ++||
>> + *   ++
>> + */
>> +static int tegra_mc_interconnect_setup(struct tegra_mc *mc)
>> +{
>> +struct icc_onecell_data *data;
>> +struct icc_node *node;
>> +unsigned int num_nodes;
>> +unsigned int i;
>> +int err;
>> +
>> +/* older device-trees don't have interconnect properties */
>> +if (!of_find_property(mc->dev->of_node, "#interconnect-cells", NULL))
>> +return 0;
>> +
>> +num_nodes = mc->soc->num_clients;
>> +
>> +data = devm_kzalloc(mc->dev, struct_size(data, nodes, num_nodes),
>> +GFP_KERNEL);
>> +if (!data)
>> +return -ENOMEM;
>> +
>> +mc->provider.dev = mc->dev;
>> +mc->provider.set = tegra_mc_icc_set;
> 
> Hmm, maybe the core should not require a set() implementation and we can
> just make it optional instead. Then the dummy function would not be needed.

Eventually this dummy function might become populated with a memory
latency allowness programming. I could add a comment into that function
in the next version, saying that it's to-be-done for now.

>> +mc->provider.data = data;
>> +mc->provider.xlate = of_icc_xlate_onecell;
>> +mc->provider.aggregate = tegra_mc_icc_aggregate;
>> +
>> +err = icc_provider_add(>provider);
>> +if (err)
>> +goto err_msg;
> 
> Nit: I am planning to re-organize some of the existing drivers to call
> icc_provider_add() after the topology is populated. Could you please move
> this after the nodes are created and linked.

Are you planning

[PATCH v9 1/4] drm/tegra: output: Don't leak OF node on error

2020-07-01 Thread Dmitry Osipenko
The OF node should be put before returning error in tegra_output_probe(),
otherwise node's refcount will be leaked.

Reviewed-by: Laurent Pinchart 
Reviewed-by: Sam Ravnborg 
Signed-off-by: Dmitry Osipenko 
---
 drivers/gpu/drm/tegra/output.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
index e36e5e7c2f69..a6a711d54e88 100644
--- a/drivers/gpu/drm/tegra/output.c
+++ b/drivers/gpu/drm/tegra/output.c
@@ -102,10 +102,10 @@ int tegra_output_probe(struct tegra_output *output)
panel = of_parse_phandle(output->of_node, "nvidia,panel", 0);
if (panel) {
output->panel = of_drm_find_panel(panel);
+   of_node_put(panel);
+
if (IS_ERR(output->panel))
return PTR_ERR(output->panel);
-
-   of_node_put(panel);
}
 
output->edid = of_get_property(output->of_node, "nvidia,edid", );
@@ -113,13 +113,12 @@ int tegra_output_probe(struct tegra_output *output)
ddc = of_parse_phandle(output->of_node, "nvidia,ddc-i2c-bus", 0);
if (ddc) {
output->ddc = of_find_i2c_adapter_by_node(ddc);
+   of_node_put(ddc);
+
if (!output->ddc) {
err = -EPROBE_DEFER;
-   of_node_put(ddc);
return err;
}
-
-   of_node_put(ddc);
}
 
output->hpd_gpio = devm_gpiod_get_from_of_node(output->dev,
-- 
2.26.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


  1   2   3   4   5   6   7   8   9   >