Re: [PATCH v2 1/9] drm/sched: Convert drm scheduler to use a work queue rather than kthread
Am 16.08.23 um 18:33 schrieb Danilo Krummrich: On 8/16/23 16:59, Christian König wrote: Am 16.08.23 um 14:30 schrieb Danilo Krummrich: On 8/16/23 16:05, Christian König wrote: Am 16.08.23 um 13:30 schrieb Danilo Krummrich: Hi Matt, On 8/11/23 04:31, Matthew Brost wrote: In XE, the new Intel GPU driver, a choice has made to have a 1 to 1 mapping between a drm_gpu_scheduler and drm_sched_entity. At first this seems a bit odd but let us explain the reasoning below. 1. In XE the submission order from multiple drm_sched_entity is not guaranteed to be the same completion even if targeting the same hardware engine. This is because in XE we have a firmware scheduler, the GuC, which allowed to reorder, timeslice, and preempt submissions. If a using shared drm_gpu_scheduler across multiple drm_sched_entity, the TDR falls apart as the TDR expects submission order == completion order. Using a dedicated drm_gpu_scheduler per drm_sched_entity solve this problem. 2. In XE submissions are done via programming a ring buffer (circular buffer), a drm_gpu_scheduler provides a limit on number of jobs, if the limit of number jobs is set to RING_SIZE / MAX_SIZE_PER_JOB we get flow control on the ring for free. In XE, where does the limitation of MAX_SIZE_PER_JOB come from? In Nouveau we currently do have such a limitation as well, but it is derived from the RING_SIZE, hence RING_SIZE / MAX_SIZE_PER_JOB would always be 1. However, I think most jobs won't actually utilize the whole ring. Well that should probably rather be RING_SIZE / MAX_SIZE_PER_JOB = hw_submission_limit (or even hw_submission_limit - 1 when the hw can't distinct full vs empty ring buffer). Not sure if I get you right, let me try to clarify what I was trying to say: I wanted to say that in Nouveau MAX_SIZE_PER_JOB isn't really limited by anything other than the RING_SIZE and hence we'd never allow more than 1 active job. But that lets the hw run dry between submissions. That is usually a pretty horrible idea for performance. Correct, that's the reason why I said it seems to be more efficient to base ring flow control on the actual size of each incoming job rather than the maximum size of a job. However, it seems to be more efficient to base ring flow control on the actual size of each incoming job rather than the worst case, namely the maximum size of a job. That doesn't sounds like a good idea to me. See we don't limit the number of submitted jobs based on the ring size, but rather we calculate the ring size based on the number of submitted jobs. My point isn't really about whether we derive the ring size from the job limit or the other way around. It's more about the job size (or its maximum size) being arbitrary. As mentioned in my reply to Matt: "In Nouveau, userspace can submit an arbitrary amount of addresses of indirect bufferes containing the ring instructions. The ring on the kernel side takes the addresses of the indirect buffers rather than the instructions themself. Hence, technically there isn't really a limit on the amount of IBs submitted by a job except for the ring size." So, my point is that I don't really want to limit the job size artificially just to be able to fit multiple jobs into the ring even if they're submitted at their "artificial" maximum size, but rather track how much of the ring the submitted job actually occupies. In other words the hw_submission_limit defines the ring size, not the other way around. And you usually want the hw_submission_limit as low as possible for good scheduler granularity and to avoid extra overhead. I don't think you really mean "as low as possible", do you? No, I do mean as low as possible or in other words as few as possible. Ideally the scheduler would submit only the minimum amount of work to the hardware to keep the hardware busy. The hardware seems to work mostly the same for all vendors, but you somehow seem to think that filling the ring is somehow beneficial which is really not the case as far as I can see. Regards, Christian. Because one really is the minimum if you want to do work at all, but as you mentioned above a job limit of one can let the ring run dry. In the end my proposal comes down to tracking the actual size of a job rather than just assuming a pre-defined maximum job size, and hence a dynamic job limit. I don't think this would hurt the scheduler granularity. In fact, it should even contribute to the desire of not letting the ring run dry even better. Especially for sequences of small jobs, where the current implementation might wrongly assume the ring is already full although actually there would still be enough space left. Christian. Otherwise your scheduler might just overwrite the ring buffer by pushing things to fast. Christian. Given that, it seems like it would be better to let the scheduler keep track of empty ring "slots" instead, such that the scheduler
Re: [RFC PATCH 1/3] drm/virtio: .release ops for virtgpu fence release
Hi, On 8/16/2023 10:05 PM, Dmitry Osipenko wrote: On 8/16/23 21:10, Kim, Dongwon wrote: Hi, On 8/14/2023 9:18 PM, Dmitry Osipenko wrote: On 7/13/23 01:44, Dongwon Kim wrote: virtio_gpu_fence_release is added to free virtio-gpu-fence upon release of dma_fence. Cc: Gerd Hoffmann Cc: Vivek Kasireddy Signed-off-by: Dongwon Kim --- drivers/gpu/drm/virtio/virtgpu_fence.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/virtio/virtgpu_fence.c b/drivers/gpu/drm/virtio/virtgpu_fence.c index f28357dbde35..ba659ac2a51d 100644 --- a/drivers/gpu/drm/virtio/virtgpu_fence.c +++ b/drivers/gpu/drm/virtio/virtgpu_fence.c @@ -63,12 +63,20 @@ static void virtio_gpu_timeline_value_str(struct dma_fence *f, char *str, (u64)atomic64_read(>drv->last_fence_id)); } +static void virtio_gpu_fence_release(struct dma_fence *f) +{ + struct virtio_gpu_fence *fence = to_virtio_gpu_fence(f); + + kfree(fence); +} + static const struct dma_fence_ops virtio_gpu_fence_ops = { .get_driver_name = virtio_gpu_get_driver_name, .get_timeline_name = virtio_gpu_get_timeline_name, .signaled = virtio_gpu_fence_signaled, .fence_value_str = virtio_gpu_fence_value_str, .timeline_value_str = virtio_gpu_timeline_value_str, + .release = virtio_gpu_fence_release, }; struct virtio_gpu_fence *virtio_gpu_fence_alloc(struct virtio_gpu_device *vgdev, This change doesn't do anything practically useful, AFAICT. The intention of this ".release" is to free virtio_gpu_fence when the last dma_fence_put is done for the associated dma fence. What makes you think that fence won't be freed otherwise? Sounds like haven't tried to check what dma_fence_release() code does, have you? Yeah, I know it frees 'struct dma_fence *f' but what about 'struct virtio_gpu_fence *fence'? This is a device specific fence that contains struct dma_fence *f. But hold on... so when fence->ops->release is called then dma_fence_free won't be called here: if (fence->ops->release) fence->ops->release(fence); else dma_fence_free(fence); In that case, I think virtio_gpu_fence_release should do "dma_fence_free(f)" before freeing virtio_gpu_fence? Am I right? Like, static void virtio_gpu_fence_release(struct dma_fence *f) { struct virtio_gpu_fence *fence = to_virtio_gpu_fence(f); dma_fence_free(f); kfree(fence); } And can you please review the second and third patches in this series as well? Thanks!
RE: [PATCH] drm/scheduler: Partially revert "drm/scheduler: track GPU active time per entity"
[Public] Hi Xinhui, That patch has been reverted on Linux mainline. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/scheduler/sched_main.c?h=v6.5-rc6=baad10973fdb442912af676de3348e80bd8fe602 Regards, Guchun > -Original Message- > From: amd-gfx On Behalf Of > xinhui pan > Sent: Thursday, August 17, 2023 1:05 PM > To: amd-...@lists.freedesktop.org > Cc: Pan, Xinhui ; dri-devel@lists.freedesktop.org; > Tuikov, Luben ; airl...@gmail.com; Koenig, > Christian ; l.st...@pengutronix.de > Subject: [PATCH] drm/scheduler: Partially revert "drm/scheduler: track GPU > active time per entity" > > This patch partially revert commit df622729ddbf ("drm/scheduler: track GPU > active time per entity") which touchs entity without any reference. > > I notice there is one memory overwritten from gpu scheduler side. > The case is like below. > A(drm_sched_main) B(vm fini) > drm_sched_job_begin drm_sched_entity_kill > //job in pending_list wait_for_completion > complete_all ... > ... kfree entity > drm_sched_get_cleanup_job > //fetch job from pending_list > access job->entity //memory overwitten > > As long as we can NOT guarantee entity is alive in this case, lets revert it > for > now. > > Signed-off-by: xinhui pan > --- > drivers/gpu/drm/scheduler/sched_main.c | 6 -- > 1 file changed, 6 deletions(-) > > diff --git a/drivers/gpu/drm/scheduler/sched_main.c > b/drivers/gpu/drm/scheduler/sched_main.c > index 602361c690c9..1b3f1a6a8514 100644 > --- a/drivers/gpu/drm/scheduler/sched_main.c > +++ b/drivers/gpu/drm/scheduler/sched_main.c > @@ -907,12 +907,6 @@ drm_sched_get_cleanup_job(struct > drm_gpu_scheduler *sched) > > spin_unlock(>job_list_lock); > > - if (job) { > - job->entity->elapsed_ns += ktime_to_ns( > - ktime_sub(job->s_fence->finished.timestamp, > - job->s_fence->scheduled.timestamp)); > - } > - > return job; > } > > -- > 2.34.1 <>
RE: [PATCH] drm/scheduler: Partially revert "drm/scheduler: track GPU active time per entity"
[AMD Official Use Only - General] Can we just add kref for entity? Or just collect such job time usage somewhere else? -Original Message- From: Pan, Xinhui Sent: Thursday, August 17, 2023 1:05 PM To: amd-...@lists.freedesktop.org Cc: Tuikov, Luben ; airl...@gmail.com; dri-devel@lists.freedesktop.org; l.st...@pengutronix.de; Koenig, Christian ; Pan, Xinhui Subject: [PATCH] drm/scheduler: Partially revert "drm/scheduler: track GPU active time per entity" This patch partially revert commit df622729ddbf ("drm/scheduler: track GPU active time per entity") which touchs entity without any reference. I notice there is one memory overwritten from gpu scheduler side. The case is like below. A(drm_sched_main) B(vm fini) drm_sched_job_begin drm_sched_entity_kill //job in pending_list wait_for_completion complete_all... ... kfree entity drm_sched_get_cleanup_job //fetch job from pending_list access job->entity //memory overwitten As long as we can NOT guarantee entity is alive in this case, lets revert it for now. Signed-off-by: xinhui pan --- drivers/gpu/drm/scheduler/sched_main.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 602361c690c9..1b3f1a6a8514 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -907,12 +907,6 @@ drm_sched_get_cleanup_job(struct drm_gpu_scheduler *sched) spin_unlock(>job_list_lock); - if (job) { - job->entity->elapsed_ns += ktime_to_ns( - ktime_sub(job->s_fence->finished.timestamp, - job->s_fence->scheduled.timestamp)); - } - return job; } -- 2.34.1
Re: [RFC PATCH 1/3] drm/virtio: .release ops for virtgpu fence release
On 8/16/23 21:10, Kim, Dongwon wrote: > Hi, > > On 8/14/2023 9:18 PM, Dmitry Osipenko wrote: >> On 7/13/23 01:44, Dongwon Kim wrote: >>> virtio_gpu_fence_release is added to free virtio-gpu-fence >>> upon release of dma_fence. >>> >>> Cc: Gerd Hoffmann >>> Cc: Vivek Kasireddy >>> Signed-off-by: Dongwon Kim >>> --- >>> drivers/gpu/drm/virtio/virtgpu_fence.c | 8 >>> 1 file changed, 8 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/virtio/virtgpu_fence.c >>> b/drivers/gpu/drm/virtio/virtgpu_fence.c >>> index f28357dbde35..ba659ac2a51d 100644 >>> --- a/drivers/gpu/drm/virtio/virtgpu_fence.c >>> +++ b/drivers/gpu/drm/virtio/virtgpu_fence.c >>> @@ -63,12 +63,20 @@ static void virtio_gpu_timeline_value_str(struct >>> dma_fence *f, char *str, >>> (u64)atomic64_read(>drv->last_fence_id)); >>> } >>> +static void virtio_gpu_fence_release(struct dma_fence *f) >>> +{ >>> + struct virtio_gpu_fence *fence = to_virtio_gpu_fence(f); >>> + >>> + kfree(fence); >>> +} >>> + >>> static const struct dma_fence_ops virtio_gpu_fence_ops = { >>> .get_driver_name = virtio_gpu_get_driver_name, >>> .get_timeline_name = virtio_gpu_get_timeline_name, >>> .signaled = virtio_gpu_fence_signaled, >>> .fence_value_str = virtio_gpu_fence_value_str, >>> .timeline_value_str = virtio_gpu_timeline_value_str, >>> + .release = virtio_gpu_fence_release, >>> }; >>> struct virtio_gpu_fence *virtio_gpu_fence_alloc(struct >>> virtio_gpu_device *vgdev, >> This change doesn't do anything practically useful, AFAICT. > > The intention of this ".release" is to free virtio_gpu_fence when the > last dma_fence_put is done for the associated dma fence. What makes you think that fence won't be freed otherwise? Sounds like haven't tried to check what dma_fence_release() code does, have you? -- Best regards, Dmitry
[PATCH] drm/scheduler: Partially revert "drm/scheduler: track GPU active time per entity"
This patch partially revert commit df622729ddbf ("drm/scheduler: track GPU active time per entity") which touchs entity without any reference. I notice there is one memory overwritten from gpu scheduler side. The case is like below. A(drm_sched_main) B(vm fini) drm_sched_job_begin drm_sched_entity_kill //job in pending_list wait_for_completion complete_all... ... kfree entity drm_sched_get_cleanup_job //fetch job from pending_list access job->entity //memory overwitten As long as we can NOT guarantee entity is alive in this case, lets revert it for now. Signed-off-by: xinhui pan --- drivers/gpu/drm/scheduler/sched_main.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 602361c690c9..1b3f1a6a8514 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -907,12 +907,6 @@ drm_sched_get_cleanup_job(struct drm_gpu_scheduler *sched) spin_unlock(>job_list_lock); - if (job) { - job->entity->elapsed_ns += ktime_to_ns( - ktime_sub(job->s_fence->finished.timestamp, - job->s_fence->scheduled.timestamp)); - } - return job; } -- 2.34.1
Re: [PATCH] Documentation/gpu: Update amdgpu documentation
On 8/17/2023 2:16 AM, Alex Deucher wrote: On Wed, Aug 16, 2023 at 12:15 AM Lijo Lazar wrote: 7957ec80ef97 ("drm/amdgpu: Add FRU sysfs nodes only if needed") moved the documentation for some of the sysfs nodes to amdgpu_fru_eeprom.c. Update the documentation accordingly. Signed-off-by: Lijo Lazar --- Documentation/gpu/amdgpu/driver-misc.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/gpu/amdgpu/driver-misc.rst b/Documentation/gpu/amdgpu/driver-misc.rst index be131e963d87..26334e54447b 100644 --- a/Documentation/gpu/amdgpu/driver-misc.rst +++ b/Documentation/gpu/amdgpu/driver-misc.rst @@ -11,19 +11,19 @@ via sysfs product_name -.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c :doc: product_name product_number -- -.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c :doc: product_name I think this should be product_number Alex Thanks, made the change while pushing. Thanks, Lijo serial_number - -.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c :doc: serial_number unique_id -- 2.25.1
Re: [PATCH 1/5] mm: move some shrinker-related function declarations to mm/internal.h
On 2023/8/16 23:01, kernel test robot wrote: Hi Qi, kernel test robot noticed the following build warnings: [auto build test WARNING on brauner-vfs/vfs.all] [also build test WARNING on linus/master v6.5-rc6 next-20230816] [cannot apply to akpm-mm/mm-everything drm-misc/drm-misc-next vfs-idmapping/for-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Qi-Zheng/mm-move-some-shrinker-related-function-declarations-to-mm-internal-h/20230816-163833 base: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.all patch link: https://lore.kernel.org/r/20230816083419.41088-2-zhengqi.arch%40bytedance.com patch subject: [PATCH 1/5] mm: move some shrinker-related function declarations to mm/internal.h config: x86_64-buildonly-randconfig-r003-20230816 (https://download.01.org/0day-ci/archive/20230816/202308162208.cqbngoer-...@intel.com/config) compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0 reproduce: (https://download.01.org/0day-ci/archive/20230816/202308162208.cqbngoer-...@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/202308162208.cqbngoer-...@intel.com/ All warnings (new ones prefixed by >>): mm/shrinker_debug.c:174:5: warning: no previous declaration for 'shrinker_debugfs_add' [-Wmissing-declarations] int shrinker_debugfs_add(struct shrinker *shrinker) ^~~~ mm/shrinker_debug.c:249:16: warning: no previous declaration for 'shrinker_debugfs_detach' [-Wmissing-declarations] struct dentry *shrinker_debugfs_detach(struct shrinker *shrinker, ^~~ mm/shrinker_debug.c:265:6: warning: no previous declaration for 'shrinker_debugfs_remove' [-Wmissing-declarations] void shrinker_debugfs_remove(struct dentry *debugfs_entry, int debugfs_id) ^~~ Compiling with W=1 will report this warning, will fix it by including "internal.h" in the mm/shrinker_debug.c. Thanks, Qi vim +/shrinker_debugfs_add +174 mm/shrinker_debug.c bbf535fd6f06b9 Roman Gushchin 2022-05-31 173 5035ebc644aec9 Roman Gushchin 2022-05-31 @174 int shrinker_debugfs_add(struct shrinker *shrinker) 5035ebc644aec9 Roman Gushchin 2022-05-31 175 { 5035ebc644aec9 Roman Gushchin 2022-05-31 176 struct dentry *entry; e33c267ab70de4 Roman Gushchin 2022-05-31 177 char buf[128]; 5035ebc644aec9 Roman Gushchin 2022-05-31 178 int id; 5035ebc644aec9 Roman Gushchin 2022-05-31 179 47a7c01c3efc65 Qi Zheng 2023-06-09 180 lockdep_assert_held(_rwsem); 5035ebc644aec9 Roman Gushchin 2022-05-31 181 5035ebc644aec9 Roman Gushchin 2022-05-31 182 /* debugfs isn't initialized yet, add debugfs entries later. */ 5035ebc644aec9 Roman Gushchin 2022-05-31 183 if (!shrinker_debugfs_root) 5035ebc644aec9 Roman Gushchin 2022-05-31 184 return 0; 5035ebc644aec9 Roman Gushchin 2022-05-31 185 5035ebc644aec9 Roman Gushchin 2022-05-31 186 id = ida_alloc(_debugfs_ida, GFP_KERNEL); 5035ebc644aec9 Roman Gushchin 2022-05-31 187 if (id < 0) 5035ebc644aec9 Roman Gushchin 2022-05-31 188 return id; 5035ebc644aec9 Roman Gushchin 2022-05-31 189 shrinker->debugfs_id = id; 5035ebc644aec9 Roman Gushchin 2022-05-31 190 e33c267ab70de4 Roman Gushchin 2022-05-31 191 snprintf(buf, sizeof(buf), "%s-%d", shrinker->name, id); 5035ebc644aec9 Roman Gushchin 2022-05-31 192 5035ebc644aec9 Roman Gushchin 2022-05-31 193 /* create debugfs entry */ 5035ebc644aec9 Roman Gushchin 2022-05-31 194 entry = debugfs_create_dir(buf, shrinker_debugfs_root); 5035ebc644aec9 Roman Gushchin 2022-05-31 195 if (IS_ERR(entry)) { 5035ebc644aec9 Roman Gushchin 2022-05-31 196 ida_free(_debugfs_ida, id); 5035ebc644aec9 Roman Gushchin 2022-05-31 197 return PTR_ERR(entry); 5035ebc644aec9 Roman Gushchin 2022-05-31 198 } 5035ebc644aec9 Roman Gushchin 2022-05-31 199 shrinker->debugfs_entry = entry; 5035ebc644aec9 Roman Gushchin 2022-05-31 200 2124f79de6a909 John Keeping 2023-04-18 201 debugfs_create_file("count", 0440, entry, shrinker, 5035ebc644aec9 Roman Gushchin 2022-05-31 202 _debugfs_count_fops); 2124f79de6a909 John Keeping 2023-04-18 203 debugfs_create_file("scan", 0220, entry, shrinker, bbf535fd6f06b9 Roman Gushchin 2022-05-31 204 _debugfs_scan_fops); 5035eb
RE: Implement svm without BO concept in xe driver
> -Original Message- > From: Dave Airlie > Sent: August 16, 2023 6:52 PM > To: Felix Kuehling > Cc: Zeng, Oak ; Christian König > ; Thomas Hellström > ; Brost, Matthew > ; maarten.lankho...@linux.intel.com; > Vishwanathapura, Niranjana ; Welty, > Brian ; Philip Yang ; intel- > x...@lists.freedesktop.org; dri-devel@lists.freedesktop.org > Subject: Re: Implement svm without BO concept in xe driver > > On Thu, 17 Aug 2023 at 08:15, Felix Kuehling wrote: > > > > On 2023-08-16 13:30, Zeng, Oak wrote: > > > I spoke with Thomas. We discussed two approaches: > > > > > > 1) make ttm_resource a central place for vram management functions such as > eviction, cgroup memory accounting. Both the BO-based driver and BO-less SVM > codes call into ttm_resource_alloc/free functions for vram allocation/free. > > > *This way BO driver and SVM driver shares the eviction/cgroup logic, > > > no > need to reimplment LRU eviction list in SVM driver. Cgroup logic should be in > ttm_resource layer. +Maarten. > > > *ttm_resource is not a perfect match for SVM to allocate vram. It is > > > still a > big overhead. The *bo* member of ttm_resource is not needed for SVM - this > might end up with invasive changes to ttm...need to look into more details > > > > Overhead is a problem. We'd want to be able to allocate, free and evict > > memory at a similar granularity as our preferred migration and page > > fault granularity, which defaults to 2MB in our SVM implementation. > > > > > > > > > > 2) svm code allocate memory directly from drm-buddy allocator, and expose > memory eviction functions from both ttm and svm so they can evict memory > from each other. For example, expose the ttm_mem_evict_first function from > ttm side so hmm/svm code can call it; expose a similar function from svm side > so > ttm can evict hmm memory. > > > > I like this option. One thing that needs some thought with this is how > > to get some semblance of fairness between the two types of clients. > > Basically how to choose what to evict. And what share of the available > > memory does each side get to use on average. E.g. an idle client may get > > all its memory evicted while a busy client may get a bigger share of the > > available memory. > > I'd also like to suggest we try to write any management/generic code > in driver agnostic way as much as possible here. I don't really see > much hw difference should be influencing it. > > I do worry about having effectively 2 LRUs here, you can't really have > two "leasts". > > Like if we hit the shrinker paths who goes first? do we shrink one > object from each side in turn? One way to solve this fairness problem is to create a driver agnostic drm_vram_mgr. Maintain a single LRU in drm_vram_mgr. Move the memory eviction/cgroups memory accounting logic from ttm_resource manager to drm_vram_mgr. Both BO-based driver and SVM driver calls to drm_vram_mgr to allocate/free memory. I am not sure whether this meets the 2M allocate/free/evict granularity requirement Felix mentioned above. SVM can allocate 2M size blocks. But BO driver should be able to allocate any arbitrary sized blocks - So the eviction is also arbitrary size. > > Also will we have systems where we can expose system SVM but userspace > may choose to not use the fine grained SVM and use one of the older > modes, will that path get emulated on top of SVM or use the BO paths? If by "older modes" you meant the gem_bo_create (such as xe_gem_create or amdgpu_gem_create), then today both amd and intel implement those interfaces using BO path. We don't have a plan to emulate that old mode on tope of SVM, afaict. Thanks, Oak > > Dave.
Re: [PATCH v2] Documentation/gpu: VM_BIND locking document
Hi Thomas, kernel test robot noticed the following build warnings: [auto build test WARNING on drm-misc/drm-misc-next] [also build test WARNING on drm/drm-next drm-tip/drm-tip linus/master v6.5-rc6 next-20230816] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Thomas-Hellstr-m/Documentation-gpu-VM_BIND-locking-document/20230816-171911 base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next patch link: https://lore.kernel.org/r/20230816091547.2982-1-thomas.hellstrom%40linux.intel.com patch subject: [PATCH v2] Documentation/gpu: VM_BIND locking document reproduce: (https://download.01.org/0day-ci/archive/20230817/202308170916.tgy7kbpm-...@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/202308170916.tgy7kbpm-...@intel.com/ All warnings (new ones prefixed by >>): >> Documentation/gpu/drm-vm-bind-locking.rst: WARNING: document isn't included >> in any toctree -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Re: [PATCH] drm/nouveau/disp: fix use-after-free in error handling of nouveau_connector_create
On Thu, Aug 17, 2023 at 12:14 AM Borislav Petkov wrote: > > On Wed, Aug 16, 2023 at 11:27:05PM +0200, Karol Herbst wrote: > > that GPU has only a `DMS-59` connector, is that right? > > No clue. How do I figure that out? > do you have one of these? https://en.wikipedia.org/wiki/DMS-59 > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette >
[PATCH v2] drm/i915/huc: silence injected failure in the load via GSC path
If we can't load the HuC due to an injected failure, we don't want to throw and error and trip CI. Using the gt_probe_error macro for logging ensure that the error is only printed if it wasn't explicitly injected. v2: keep the line to less than 100 characters (checkpatch). Link: https://gitlab.freedesktop.org/drm/intel/-/issues/7061 Signed-off-by: Daniele Ceraolo Spurio Reviewed-by: Andi Shyti #v1 --- drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c index f89a1f80f50e..bb58fa9579b8 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c @@ -9,6 +9,7 @@ #include #include "gem/i915_gem_lmem.h" +#include "gt/intel_gt_print.h" #include "i915_drv.h" #include "gt/intel_gt.h" @@ -156,7 +157,8 @@ static int i915_pxp_tee_component_bind(struct device *i915_kdev, { struct drm_i915_private *i915 = kdev_to_i915(i915_kdev); struct intel_pxp *pxp = i915->pxp; - struct intel_uc *uc = >ctrl_gt->uc; + struct intel_gt *gt = pxp->ctrl_gt; + struct intel_uc *uc = >uc; intel_wakeref_t wakeref; int ret = 0; @@ -176,7 +178,7 @@ static int i915_pxp_tee_component_bind(struct device *i915_kdev, /* load huc via pxp */ ret = intel_huc_fw_load_and_auth_via_gsc(>huc); if (ret < 0) - drm_err(>drm, "failed to load huc via gsc %d\n", ret); + gt_probe_error(gt, "failed to load huc via gsc %d\n", ret); } } -- 2.41.0
Re: Implement svm without BO concept in xe driver
On Thu, 17 Aug 2023 at 08:15, Felix Kuehling wrote: > > On 2023-08-16 13:30, Zeng, Oak wrote: > > I spoke with Thomas. We discussed two approaches: > > > > 1) make ttm_resource a central place for vram management functions such as > > eviction, cgroup memory accounting. Both the BO-based driver and BO-less > > SVM codes call into ttm_resource_alloc/free functions for vram > > allocation/free. > > *This way BO driver and SVM driver shares the eviction/cgroup logic, > > no need to reimplment LRU eviction list in SVM driver. Cgroup logic should > > be in ttm_resource layer. +Maarten. > > *ttm_resource is not a perfect match for SVM to allocate vram. It is > > still a big overhead. The *bo* member of ttm_resource is not needed for SVM > > - this might end up with invasive changes to ttm...need to look into more > > details > > Overhead is a problem. We'd want to be able to allocate, free and evict > memory at a similar granularity as our preferred migration and page > fault granularity, which defaults to 2MB in our SVM implementation. > > > > > > 2) svm code allocate memory directly from drm-buddy allocator, and expose > > memory eviction functions from both ttm and svm so they can evict memory > > from each other. For example, expose the ttm_mem_evict_first function from > > ttm side so hmm/svm code can call it; expose a similar function from svm > > side so ttm can evict hmm memory. > > I like this option. One thing that needs some thought with this is how > to get some semblance of fairness between the two types of clients. > Basically how to choose what to evict. And what share of the available > memory does each side get to use on average. E.g. an idle client may get > all its memory evicted while a busy client may get a bigger share of the > available memory. I'd also like to suggest we try to write any management/generic code in driver agnostic way as much as possible here. I don't really see much hw difference should be influencing it. I do worry about having effectively 2 LRUs here, you can't really have two "leasts". Like if we hit the shrinker paths who goes first? do we shrink one object from each side in turn? Also will we have systems where we can expose system SVM but userspace may choose to not use the fine grained SVM and use one of the older modes, will that path get emulated on top of SVM or use the BO paths? Dave.
Re: [PATCH] drm/nouveau/disp: fix use-after-free in error handling of nouveau_connector_create
On Wed, Aug 16, 2023 at 11:27:05PM +0200, Karol Herbst wrote: > that GPU has only a `DMS-59` connector, is that right? No clue. How do I figure that out? -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: Implement svm without BO concept in xe driver
On 2023-08-16 13:30, Zeng, Oak wrote: I spoke with Thomas. We discussed two approaches: 1) make ttm_resource a central place for vram management functions such as eviction, cgroup memory accounting. Both the BO-based driver and BO-less SVM codes call into ttm_resource_alloc/free functions for vram allocation/free. *This way BO driver and SVM driver shares the eviction/cgroup logic, no need to reimplment LRU eviction list in SVM driver. Cgroup logic should be in ttm_resource layer. +Maarten. *ttm_resource is not a perfect match for SVM to allocate vram. It is still a big overhead. The *bo* member of ttm_resource is not needed for SVM - this might end up with invasive changes to ttm...need to look into more details Overhead is a problem. We'd want to be able to allocate, free and evict memory at a similar granularity as our preferred migration and page fault granularity, which defaults to 2MB in our SVM implementation. 2) svm code allocate memory directly from drm-buddy allocator, and expose memory eviction functions from both ttm and svm so they can evict memory from each other. For example, expose the ttm_mem_evict_first function from ttm side so hmm/svm code can call it; expose a similar function from svm side so ttm can evict hmm memory. I like this option. One thing that needs some thought with this is how to get some semblance of fairness between the two types of clients. Basically how to choose what to evict. And what share of the available memory does each side get to use on average. E.g. an idle client may get all its memory evicted while a busy client may get a bigger share of the available memory. Regards, Felix Today we don't know which approach is better. I will work on some prove of concept codes, starting with #1 approach firstly. Btw, I talked with application engineers and they said most applications actually use a mixture of gem_bo create and malloc, so we definitely need to solve this problem. Cheers, Oak -Original Message- From: Christian König Sent: August 16, 2023 2:06 AM To: Zeng, Oak ; Felix Kuehling ; Thomas Hellström ; Brost, Matthew ; Vishwanathapura, Niranjana ; Welty, Brian ; Philip Yang ; intel...@lists.freedesktop.org; dri- de...@lists.freedesktop.org Subject: Re: Implement svm without BO concept in xe driver Hi Oak, yeah, I completely agree with you and Felix. The main problem here is getting the memory pressure visible on both sides. At the moment I have absolutely no idea how to handle that, maybe something like the ttm_resource object shared between TTM and HMM? Regards, Christian. Am 16.08.23 um 05:47 schrieb Zeng, Oak: Hi Felix, It is great to hear from you! When I implement the HMM-based SVM for intel devices, I found this interesting problem: HMM uses struct page based memory management scheme which is completely different against the BO/TTM style memory management philosophy. Writing SVM code upon the BO/TTM concept seems overkill and awkward. So I thought we better make the SVM code BO-less and TTM-less. But on the other hand, currently vram eviction and cgroup memory accounting are all hooked to the TTM layer, which means a TTM-less SVM driver won't be able to evict vram allocated through TTM/gpu_vram_mgr. Ideally HMM migration should use drm-buddy for vram allocation, but we need to solve this TTM/HMM mutual eviction problem as you pointed out (I am working with application engineers to figure out whether mutual eviction can truly benefit applications). Maybe we can implement a TTM-less vram management block which can be shared b/t the HMM-based driver and the BO- based driver: * allocate/free memory from drm-buddy, buddy-block based * memory eviction logics, allow driver to specify which allocation is evictable * memory accounting, cgroup logic Maybe such a block can be placed at drm layer (say, call it drm_vram_mgr for now), so it can be shared b/t amd and intel. So I involved amd folks. Today both amd and intel-xe driver implemented a TTM-based vram manager which doesn't serve above design goal. Once the drm_vram_mgr is implemented, both amd and intel's BO-based/TTM-based vram manager, and the HMM-based vram manager can call into this drm-vram-mgr. Thanks again, Oak -Original Message- From: Felix Kuehling Sent: August 15, 2023 6:17 PM To: Zeng, Oak ; Thomas Hellström ; Brost, Matthew ; Vishwanathapura, Niranjana ; Welty, Brian ; Christian König ; Philip Yang ; intel...@lists.freedesktop.org; dri- de...@lists.freedesktop.org Subject: Re: Implement svm without BO concept in xe driver Hi Oak, I'm not sure what you're looking for from AMD? Are we just CC'ed FYI? Or are you looking for comments about * Our plans for VRAM management with HMM * Our experience with BO-based VRAM management * Something else? IMO, having separate memory pools for HMM and TTM is a non-starter for AMD. We need access to the full VRAM in either of the APIs for
Re: [PATCH] drm/nouveau/disp: fix use-after-free in error handling of nouveau_connector_create
On Wed, Aug 16, 2023 at 5:13 PM Borislav Petkov wrote: > > On Wed, Aug 16, 2023 at 04:57:28PM +0200, Karol Herbst wrote: > > Do you have any connectors listed in "/sys/class/drm"? > > tree /sys/class/drm/ > /sys/class/drm/ > ├── card0 -> ../../devices/pci:00/:00:02.0/:03:00.0/drm/card0 > ├── renderD128 -> > ../../devices/pci:00/:00:02.0/:03:00.0/drm/renderD128 > └── version > > > Also, mind sharing your vbios.rom file from > > "/sys/kernel/debug/dri/0/vbios.rom"? > > Attached. that GPU has only a `DMS-59` connector, is that right? I have such a GPU myself, so maybe I can trigger that bug there, let's see.. > > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH -next v3] drm/amd/amdgpu: Use kmemdup to simplify kmalloc and memcpy logic
Applied. Thanks! Alex On Tue, Aug 15, 2023 at 10:20 PM Chen Jiahao wrote: > > Using kmemdup() helper function rather than implementing it again > with kmalloc() + memcpy(), which improves the code readability. > > Signed-off-by: Chen Jiahao > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 11 ++- > 1 file changed, 2 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c > index d34037b85cf8..7473a42f7d45 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c > @@ -264,16 +264,9 @@ struct edid *amdgpu_connector_edid(struct drm_connector > *connector) > static struct edid * > amdgpu_connector_get_hardcoded_edid(struct amdgpu_device *adev) > { > - struct edid *edid; > - > if (adev->mode_info.bios_hardcoded_edid) { > - edid = kmalloc(adev->mode_info.bios_hardcoded_edid_size, > GFP_KERNEL); > - if (edid) { > - memcpy((unsigned char *)edid, > - (unsigned char > *)adev->mode_info.bios_hardcoded_edid, > - adev->mode_info.bios_hardcoded_edid_size); > - return edid; > - } > + return kmemdup((unsigned char > *)adev->mode_info.bios_hardcoded_edid, > + adev->mode_info.bios_hardcoded_edid_size, > GFP_KERNEL); > } > return NULL; > } > -- > 2.34.1 >
Re: [PATCH] drm/nouveau/disp: fix use-after-free in error handling of nouveau_connector_create
Reviewed-by: Lyude Paul On Mon, 2023-08-14 at 16:49 +0200, Karol Herbst wrote: > We can't simply free the connector after calling drm_connector_init on it. > We need to clean up the drm side first. > > It might not fix all regressions from 2b5d1c29f6c4 ("drm/nouveau/disp: > PIOR DP uses GPIO for HPD, not PMGR AUX interrupts"), but at least it > fixes a memory corruption in error handling related to that commit. > > Link: > https://lore.kernel.org/lkml/20230806213107.GFZNARG6moWpFuSJ9W@fat_crate.local/ > Fixes: 95983aea8003 ("drm/nouveau/disp: add connector class") > Signed-off-by: Karol Herbst > --- > drivers/gpu/drm/nouveau/nouveau_connector.c | 11 +++ > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c > b/drivers/gpu/drm/nouveau/nouveau_connector.c > index a2e0033e8a260..622f6eb9a8bfd 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_connector.c > +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c > @@ -1408,8 +1408,7 @@ nouveau_connector_create(struct drm_device *dev, > ret = nvif_conn_ctor(>disp, nv_connector->base.name, > nv_connector->index, >_connector->conn); > if (ret) { > - kfree(nv_connector); > - return ERR_PTR(ret); > + goto drm_conn_err; > } > > ret = nvif_conn_event_ctor(_connector->conn, "kmsHotplug", > @@ -1426,8 +1425,7 @@ nouveau_connector_create(struct drm_device *dev, > if (ret) { > nvif_event_dtor(_connector->hpd); > nvif_conn_dtor(_connector->conn); > - kfree(nv_connector); > - return ERR_PTR(ret); > + goto drm_conn_err; > } > } > } > @@ -1475,4 +1473,9 @@ nouveau_connector_create(struct drm_device *dev, > > drm_connector_register(connector); > return connector; > + > +drm_conn_err: > + drm_connector_cleanup(connector); > + kfree(nv_connector); > + return ERR_PTR(ret); > } -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat
Re: [PATCH] Documentation/gpu: Update amdgpu documentation
On Wed, Aug 16, 2023 at 12:15 AM Lijo Lazar wrote: > > 7957ec80ef97 ("drm/amdgpu: Add FRU sysfs nodes only if needed") moved > the documentation for some of the sysfs nodes to amdgpu_fru_eeprom.c. > Update the documentation accordingly. > > Signed-off-by: Lijo Lazar > --- > Documentation/gpu/amdgpu/driver-misc.rst | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/Documentation/gpu/amdgpu/driver-misc.rst > b/Documentation/gpu/amdgpu/driver-misc.rst > index be131e963d87..26334e54447b 100644 > --- a/Documentation/gpu/amdgpu/driver-misc.rst > +++ b/Documentation/gpu/amdgpu/driver-misc.rst > @@ -11,19 +11,19 @@ via sysfs > product_name > > > -.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c > :doc: product_name > > product_number > -- > > -.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c > :doc: product_name I think this should be product_number Alex > > serial_number > - > > -.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c > :doc: serial_number > > unique_id > -- > 2.25.1 >
Re: [PATCH 2/2][next] nouveau/svm: Split assignment from if conditional
On Wed, Aug 16, 2023 at 12:05:06PM -0600, Gustavo A. R. Silva wrote: > Fix checkpatch.pl ERROR: do not use assignment in if condition. > > Signed-off-by: Gustavo A. R. Silva Reviewed-by: Kees Cook -- Kees Cook
Re: [PATCH 1/2][next] nouveau/svm: Replace one-element array with flexible-array member in struct nouveau_svm
On Wed, Aug 16, 2023 at 12:04:06PM -0600, Gustavo A. R. Silva wrote: > One-element and zero-length arrays are deprecated. So, replace > one-element array in struct nouveau_svm with flexible-array member. > > This results in no differences in binary output. > > Link: https://github.com/KSPP/linux/issues/338 > Signed-off-by: Gustavo A. R. Silva Reviewed-by: Kees Cook -- Kees Cook
[linux-next:master] BUILD REGRESSION ef66bf8aeb91fd331cf8f5dca8f9d7bca9ab2849
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master branch HEAD: ef66bf8aeb91fd331cf8f5dca8f9d7bca9ab2849 Add linux-next specific files for 20230816 Error/Warning reports: https://lore.kernel.org/oe-kbuild-all/202307281049.40t8s0uv-...@intel.com https://lore.kernel.org/oe-kbuild-all/202307301850.i9xfnwt6-...@intel.com https://lore.kernel.org/oe-kbuild-all/202308111853.isf5a6vc-...@intel.com https://lore.kernel.org/oe-kbuild-all/202308112307.tpmybd3l-...@intel.com https://lore.kernel.org/oe-kbuild-all/202308112326.ajavwcoc-...@intel.com https://lore.kernel.org/oe-kbuild-all/202308162234.y7j8jeif-...@intel.com https://lore.kernel.org/oe-kbuild-all/202308170007.ozhdwitj-...@intel.com https://lore.kernel.org/oe-kbuild-all/202308170206.fzg3v1gy-...@intel.com https://lore.kernel.org/oe-kbuild-all/202308170227.ymiflmbt-...@intel.com Error/Warning: (recently discovered and may have been fixed) ../lib/gcc/loongarch64-linux/12.3.0/plugin/include/config/loongarch/loongarch-opts.h:31:10: fatal error: loongarch-def.h: No such file or directory Documentation/gpu/rfc/i915_scheduler.rst:138: WARNING: Unknown directive type "c:namespace-push". Documentation/gpu/rfc/i915_scheduler.rst:143: WARNING: Unknown directive type "c:namespace-pop". Warning: kernel/Kconfig.kexec references a file that doesn't exist: file:Documentation/s390/zfcpdump.rst arch/csky/include/asm/ptrace.h:100:11: error: expected ';' before 'void' arch/csky/include/asm/ptrace.h:99:11: error: expected ';' before 'int' arch/csky/include/asm/traps.h:43:11: error: expected ';' before 'void' arch/loongarch/kernel/asm-offsets.c:172:6: warning: no previous prototype for 'output_thread_lbt_defines' [-Wmissing-prototypes] drivers/gpu/drm/drm_gpuva_mgr.c:1079:32: warning: variable 'prev' set but not used [-Wunused-but-set-variable] drivers/gpu/drm/drm_gpuva_mgr.c:1079:39: warning: variable 'prev' set but not used [-Wunused-but-set-variable] drivers/gpu/drm/tests/drm_kunit_helpers.c:172: warning: expecting prototype for drm_kunit_helper_context_alloc(). Prototype was for drm_kunit_helper_acquire_ctx_alloc() instead drivers/media/pci/intel/ivsc/mei_csi.c:342:10: error: call to undeclared function 'v4l2_subdev_get_try_format'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] drivers/media/pci/intel/ivsc/mei_csi.c:342:10: error: incompatible integer to pointer conversion returning 'int' from a function with result type 'struct v4l2_mbus_framefmt *' [-Wint-conversion] drivers/media/pci/intel/ivsc/mei_csi.c:360:14: error: incompatible integer to pointer conversion assigning to 'struct v4l2_mbus_framefmt *' from 'int' [-Wint-conversion] drivers/video/backlight/lp855x_bl.c:252:11: warning: variable 'ret' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] fs/fuse/dir.c:353:6: warning: no previous declaration for 'fuse_valid_size' [-Wmissing-declarations] include/linux/build_bug.h:16:51: error: bit-field '' width not an integer constant kernel/bpf/map_iter.c:201:17: warning: no previous declaration for 'bpf_map_sum_elem_count' [-Wmissing-declarations] net/bpf/test_run.c:558:15: warning: no previous declaration for 'bpf_fentry_test_sinfo' [-Wmissing-declarations] net/bpf/test_run.c:559:15: warning: no previous declaration for 'bpf_fentry_test_sinfo' [-Wmissing-declarations] net/bpf/test_run.c:568:17: warning: no previous declaration for 'bpf_modify_return_test2' [-Wmissing-declarations] Unverified Error/Warning (likely false positive, please contact us if interested): drivers/input/touchscreen/iqs7211.c:1761 iqs7211_parse_cycles() error: buffer overflow 'cycle_alloc[0]' 2 <= 41 drivers/mtd/nand/raw/qcom_nandc.c:2590 qcom_op_cmd_mapping() error: uninitialized symbol 'ret'. drivers/mtd/nand/raw/qcom_nandc.c:3017 qcom_check_op() warn: was && intended here instead of ||? drivers/net/ethernet/mellanox/mlx5/core/lib/devcom.c:264 mlx5_devcom_send_event() warn: variable dereferenced before IS_ERR check 'devcom' (see line 259) drivers/net/wireless/mediatek/mt76/mt792x_mac.c:362 mt792x_pm_power_save_work() warn: can 'dev->fw_assert' even be NULL? drivers/pwm/pwm-atmel.c:479 atmel_pwm_enable_clk_if_on() warn: missing error code 'ret' drivers/regulator/max77857-regulator.c:430:28: sparse: sparse: symbol 'max77857_id' was not declared. Should it be static? drivers/rtc/rtc-pcf2127.c:1063 pcf2127_enable_ts() warn: missing error code? 'ret' kernel/workqueue.c:325:40: sparse: sparse: duplicate [noderef] kernel/workqueue.c:325:40: sparse: sparse: multiple address spaces given: __percpu & __rcu lib/crypto/mpi/mpi-inv.c:34:15: warning: variable 'k' set but not used [-Wunused-but-set-variable] net/xdp/xsk.c:696 xsk_build_skb() error: 'skb' dereferencing possible ERR_PTR() sh4-linux-gcc: internal compiler error: Segmentation fault signal terminated program cc1 {standard input}: Warning: end of file
[PATCH] drm: pl111: fix clang -Wvoid-pointer-to-enum-cast warning
When building with clang 18 I see the following warnings: |1. drivers/gpu/drm/pl111/pl111_versatile.c:487:24: warning: cast to smaller | integer type 'enum versatile_clcd' from 'const void *' [-Wvoid-pointer-to-enum-cast] | 487 | versatile_clcd_type = (enum versatile_clcd)clcd_id->data; - |2. drivers/gpu/drm/pl111/pl111_versatile.c:507:26: warning: cast to smaller | integer type 'enum versatile_clcd' from 'const void *' [-Wvoid-pointer-to-enum-cast] | 507 | versatile_clcd_type = (enum versatile_clcd)clcd_id->data; This is due to the fact that `clcd_id->data` is a void* while `enum versatile_clcd` has the size of an int. Cast `clcd_id->data` to a uintptr_t to silence the above warning for clang builds using W=1 Link: https://github.com/ClangBuiltLinux/linux/issues/1910 Reported-by: Nathan Chancellor Signed-off-by: Justin Stitt --- drivers/gpu/drm/pl111/pl111_versatile.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c b/drivers/gpu/drm/pl111/pl111_versatile.c index 00c3ebd32359..d6fd9e51377e 100644 --- a/drivers/gpu/drm/pl111/pl111_versatile.c +++ b/drivers/gpu/drm/pl111/pl111_versatile.c @@ -484,7 +484,7 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv) return 0; } - versatile_clcd_type = (enum versatile_clcd)clcd_id->data; + versatile_clcd_type = (enum versatile_clcd)(uintptr_t)clcd_id->data; /* Versatile Express special handling */ if (versatile_clcd_type == VEXPRESS_CLCD_V2M) { @@ -504,7 +504,7 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv) np = of_find_matching_node_and_match(NULL, impd1_clcd_of_match, _id); if (np) - versatile_clcd_type = (enum versatile_clcd)clcd_id->data; + versatile_clcd_type = (enum versatile_clcd)(uintptr_t)clcd_id->data; } map = syscon_node_to_regmap(np); --- base-commit: 2ccdd1b13c591d306f0401d98dedc4bdcd02b421 change-id: 20230816-void-drivers-gpu-drm-pl111-pl111_versatile-43b109cfa7ad Best regards, -- Justin Stitt
[pull] amdgpu drm-fixes-6.5
Hi Dave, Daniel, Fixes for 6.5. The following changes since commit 2ccdd1b13c591d306f0401d98dedc4bdcd02b421: Linux 6.5-rc6 (2023-08-13 11:29:55 -0700) are available in the Git repository at: https://gitlab.freedesktop.org/agd5f/linux.git tags/amd-drm-fixes-6.5-2023-08-16 for you to fetch changes up to 6ecc10295abb2fdd9c21dd17b34e4cacfd829cd4: Revert "Revert "drm/amdgpu/display: change pipe policy for DCN 2.0"" (2023-08-16 15:46:40 -0400) amd-drm-fixes-6.5-2023-08-16: amdgpu: - SMU 13.x fixes - Fix mcbp parameter for gfx9 - SMU 11.x fixes - Temporary fix for large numbers of XCP partitions - S0ix fixes - DCN 2.0 fix Alex Deucher (1): Revert "Revert "drm/amdgpu/display: change pipe policy for DCN 2.0"" Asad Kamal (1): drm/amd/pm: Update pci link width for smu v13.0.6 James Zhu (1): drm/amdgpu: skip xcp drm device allocation when out of drm resource Jiadong Zhu (1): drm/amdgpu: disable mcbp if parameter zero is set Kenneth Feng (1): drm/amd/pm: disallow the fan setting if there is no fan on smu 13.0.0 Lijo Lazar (1): drm/amd/pm: Fix temperature unit of SMU v13.0.6 Mario Limonciello (1): drm/amd: flush any delayed gfxoff on suspend entry Tim Huang (1): drm/amdgpu: skip fence GFX interrupts disable/enable for S0ix Umio Yasuno (1): drm/amdgpu/pm: fix throttle_status for other than MP1 11.0.7 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 +++--- drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 41 -- drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c| 9 + drivers/gpu/drm/amd/amdgpu/amdgpu_ring_mux.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_xcp.c| 13 ++- drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 9 - .../gpu/drm/amd/display/dc/dcn20/dcn20_resource.c | 2 +- .../drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c| 14 .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 4 +++ .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c | 19 +++--- 10 files changed, 93 insertions(+), 30 deletions(-)
[PATCH] drm/repaper: fix -Wvoid-pointer-to-enum-cast warning
When building with clang 18 I see the following warning: | drivers/gpu/drm/tiny/repaper.c:952:11: warning: cast to smaller integer | type 'enum repaper_model' from 'const void *' [-Wvoid-pointer-to-enum-cast] | 952 | model = (enum repaper_model)match; | This is due to the fact that `match` is a void* while `enum repaper_model` has the size of an int. Add uintptr_t cast to silence clang warning while also keeping enum cast for readability and consistency with other `model` assignment just a few lines below: | model = (enum repaper_model)spi_id->driver_data; Link: https://github.com/ClangBuiltLinux/linux/issues/1910 Reported-by: Nathan Chancellor Signed-off-by: Justin Stitt --- drivers/gpu/drm/tiny/repaper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c index c2677d081a7b..165f2099e7d8 100644 --- a/drivers/gpu/drm/tiny/repaper.c +++ b/drivers/gpu/drm/tiny/repaper.c @@ -949,7 +949,7 @@ static int repaper_probe(struct spi_device *spi) match = device_get_match_data(dev); if (match) { - model = (enum repaper_model)match; + model = (enum repaper_model)(uintptr_t)match; } else { spi_id = spi_get_device_id(spi); model = (enum repaper_model)spi_id->driver_data; --- base-commit: 2ccdd1b13c591d306f0401d98dedc4bdcd02b421 change-id: 20230816-void-drivers-gpu-drm-tiny-repaper-a08321cd99d7 Best regards, -- Justin Stitt
[Bug 217664] Laptop doesnt wake up from suspend mode.
https://bugzilla.kernel.org/show_bug.cgi?id=217664 --- Comment #13 from Alex Deucher (alexdeuc...@gmail.com) --- Are any of the external display connectors driven by the APU? If so, do any of those work correctly on resume? -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [RFC PATCH 1/3] drm/virtio: .release ops for virtgpu fence release
Hi, On 8/14/2023 9:18 PM, Dmitry Osipenko wrote: On 7/13/23 01:44, Dongwon Kim wrote: virtio_gpu_fence_release is added to free virtio-gpu-fence upon release of dma_fence. Cc: Gerd Hoffmann Cc: Vivek Kasireddy Signed-off-by: Dongwon Kim --- drivers/gpu/drm/virtio/virtgpu_fence.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/virtio/virtgpu_fence.c b/drivers/gpu/drm/virtio/virtgpu_fence.c index f28357dbde35..ba659ac2a51d 100644 --- a/drivers/gpu/drm/virtio/virtgpu_fence.c +++ b/drivers/gpu/drm/virtio/virtgpu_fence.c @@ -63,12 +63,20 @@ static void virtio_gpu_timeline_value_str(struct dma_fence *f, char *str, (u64)atomic64_read(>drv->last_fence_id)); } +static void virtio_gpu_fence_release(struct dma_fence *f) +{ + struct virtio_gpu_fence *fence = to_virtio_gpu_fence(f); + + kfree(fence); +} + static const struct dma_fence_ops virtio_gpu_fence_ops = { .get_driver_name = virtio_gpu_get_driver_name, .get_timeline_name = virtio_gpu_get_timeline_name, .signaled= virtio_gpu_fence_signaled, .fence_value_str = virtio_gpu_fence_value_str, .timeline_value_str = virtio_gpu_timeline_value_str, + .release = virtio_gpu_fence_release, }; struct virtio_gpu_fence *virtio_gpu_fence_alloc(struct virtio_gpu_device *vgdev, This change doesn't do anything practically useful, AFAICT. The intention of this ".release" is to free virtio_gpu_fence when the last dma_fence_put is done for the associated dma fence.
[PATCH 2/2][next] nouveau/svm: Split assignment from if conditional
Fix checkpatch.pl ERROR: do not use assignment in if condition. Signed-off-by: Gustavo A. R. Silva --- drivers/gpu/drm/nouveau/nouveau_svm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c index 00444ad82d18..cc03e0c22ff3 100644 --- a/drivers/gpu/drm/nouveau/nouveau_svm.c +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c @@ -1063,7 +1063,8 @@ nouveau_svm_init(struct nouveau_drm *drm) if (drm->client.device.info.family > NV_DEVICE_INFO_V0_PASCAL) return; - if (!(drm->svm = svm = kzalloc(struct_size(drm->svm, buffer, 1), GFP_KERNEL))) + drm->svm = svm = kzalloc(struct_size(drm->svm, buffer, 1), GFP_KERNEL); + if (!drm->svm) return; drm->svm->drm = drm; -- 2.34.1
[PATCH 1/2][next] nouveau/svm: Replace one-element array with flexible-array member in struct nouveau_svm
One-element and zero-length arrays are deprecated. So, replace one-element array in struct nouveau_svm with flexible-array member. This results in no differences in binary output. Link: https://github.com/KSPP/linux/issues/338 Signed-off-by: Gustavo A. R. Silva --- drivers/gpu/drm/nouveau/nouveau_svm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c index 186351ecf72f..00444ad82d18 100644 --- a/drivers/gpu/drm/nouveau/nouveau_svm.c +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c @@ -67,7 +67,7 @@ struct nouveau_svm { struct nouveau_svmm *svmm; } **fault; int fault_nr; - } buffer[1]; + } buffer[]; }; #define FAULT_ACCESS_READ 0 @@ -1063,7 +1063,7 @@ nouveau_svm_init(struct nouveau_drm *drm) if (drm->client.device.info.family > NV_DEVICE_INFO_V0_PASCAL) return; - if (!(drm->svm = svm = kzalloc(sizeof(*drm->svm), GFP_KERNEL))) + if (!(drm->svm = svm = kzalloc(struct_size(drm->svm, buffer, 1), GFP_KERNEL))) return; drm->svm->drm = drm; -- 2.34.1
[PATCH 0/2][next] nouveau/svm: Replace one-element array with flexible-array member
This small series aims to replace a one-element array with a flexible-array member in struct nouveau_svm. And, while at it, fix a checkpatch.pl error. Gustavo A. R. Silva (2): nouveau/svm: Replace one-element array with flexible-array member in struct nouveau_svm nouveau/svm: Split assignment from if conditional drivers/gpu/drm/nouveau/nouveau_svm.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) -- 2.34.1
[Bug 217664] Laptop doesnt wake up from suspend mode.
https://bugzilla.kernel.org/show_bug.cgi?id=217664 --- Comment #12 from Alex Deucher (alexdeuc...@gmail.com) --- The display mux is a switch that controls what GPU the built in display is routed to (dGPU or APU). If it's not set correctly it can route the display to the wrong GPU. AFAIK, it should be handled by the nvidia driver. I'm not sure if they support this or not on Linux. You indicated that the system is still accessible after resume, can you get the dmesg output after resume? -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
RE: Implement svm without BO concept in xe driver
I spoke with Thomas. We discussed two approaches: 1) make ttm_resource a central place for vram management functions such as eviction, cgroup memory accounting. Both the BO-based driver and BO-less SVM codes call into ttm_resource_alloc/free functions for vram allocation/free. *This way BO driver and SVM driver shares the eviction/cgroup logic, no need to reimplment LRU eviction list in SVM driver. Cgroup logic should be in ttm_resource layer. +Maarten. *ttm_resource is not a perfect match for SVM to allocate vram. It is still a big overhead. The *bo* member of ttm_resource is not needed for SVM - this might end up with invasive changes to ttm...need to look into more details 2) svm code allocate memory directly from drm-buddy allocator, and expose memory eviction functions from both ttm and svm so they can evict memory from each other. For example, expose the ttm_mem_evict_first function from ttm side so hmm/svm code can call it; expose a similar function from svm side so ttm can evict hmm memory. Today we don't know which approach is better. I will work on some prove of concept codes, starting with #1 approach firstly. Btw, I talked with application engineers and they said most applications actually use a mixture of gem_bo create and malloc, so we definitely need to solve this problem. Cheers, Oak > -Original Message- > From: Christian König > Sent: August 16, 2023 2:06 AM > To: Zeng, Oak ; Felix Kuehling ; > Thomas Hellström ; Brost, Matthew > ; Vishwanathapura, Niranjana > ; Welty, Brian ; > Philip Yang ; intel...@lists.freedesktop.org; dri- > de...@lists.freedesktop.org > Subject: Re: Implement svm without BO concept in xe driver > > Hi Oak, > > yeah, I completely agree with you and Felix. The main problem here is > getting the memory pressure visible on both sides. > > At the moment I have absolutely no idea how to handle that, maybe > something like the ttm_resource object shared between TTM and HMM? > > Regards, > Christian. > > Am 16.08.23 um 05:47 schrieb Zeng, Oak: > > Hi Felix, > > > > It is great to hear from you! > > > > When I implement the HMM-based SVM for intel devices, I found this > interesting problem: HMM uses struct page based memory management scheme > which is completely different against the BO/TTM style memory management > philosophy. Writing SVM code upon the BO/TTM concept seems overkill and > awkward. So I thought we better make the SVM code BO-less and TTM-less. But > on the other hand, currently vram eviction and cgroup memory accounting are > all > hooked to the TTM layer, which means a TTM-less SVM driver won't be able to > evict vram allocated through TTM/gpu_vram_mgr. > > > > Ideally HMM migration should use drm-buddy for vram allocation, but we need > to solve this TTM/HMM mutual eviction problem as you pointed out (I am > working with application engineers to figure out whether mutual eviction can > truly benefit applications). Maybe we can implement a TTM-less vram > management block which can be shared b/t the HMM-based driver and the BO- > based driver: > > * allocate/free memory from drm-buddy, buddy-block based > > * memory eviction logics, allow driver to specify which allocation is > > evictable > > * memory accounting, cgroup logic > > > > Maybe such a block can be placed at drm layer (say, call it drm_vram_mgr for > now), so it can be shared b/t amd and intel. So I involved amd folks. Today > both > amd and intel-xe driver implemented a TTM-based vram manager which doesn't > serve above design goal. Once the drm_vram_mgr is implemented, both amd > and intel's BO-based/TTM-based vram manager, and the HMM-based vram > manager can call into this drm-vram-mgr. > > > > Thanks again, > > Oak > > > >> -Original Message- > >> From: Felix Kuehling > >> Sent: August 15, 2023 6:17 PM > >> To: Zeng, Oak ; Thomas Hellström > >> ; Brost, Matthew > >> ; Vishwanathapura, Niranjana > >> ; Welty, Brian > ; > >> Christian König ; Philip Yang > >> ; intel...@lists.freedesktop.org; dri- > >> de...@lists.freedesktop.org > >> Subject: Re: Implement svm without BO concept in xe driver > >> > >> Hi Oak, > >> > >> I'm not sure what you're looking for from AMD? Are we just CC'ed FYI? Or > >> are you looking for comments about > >> > >>* Our plans for VRAM management with HMM > >>* Our experience with BO-based VRAM management > >>* Something else? > >> > >> IMO, having separate memory pools for HMM and TTM is a non-starter for > >> AMD. We need access to the full VRAM in either of the APIs for it to be > >> useful. That also means, we need to handle memory pressure in both > >> directions. That's one of the main reasons we went with the BO-based > >> approach initially. I think in the long run, using the buddy allocator, > >> or the amdgpu_vram_mgr directly for HMM migrations would be better, > >> assuming we can handle memory pressure in both directions between HMM > >> and TTM sharing the same pool
Re: [PATCH v2 00/12] drm/bridge: tc358768: Fixes and timings improvements
On 16/08/2023 14:25, Tomi Valkeinen wrote: This series contains various fixes and cleanups for TC358768. The target of this work is to get TC358768 working on Toradex's AM62 based board, which has the following display pipeline: AM62 DPI -> TC358768 -> LT8912B -> HDMI connector The main thing the series does is to improve the DSI HSW, HFP and VSDly calculations. Tomi Signed-off-by: Tomi Valkeinen --- Changes in v2: - Add "drm/tegra: rgb: Parameterize V- and H-sync polarities" so that Tegra can configure the polarities correctly. - Add "drm/bridge: tc358768: Default to positive h/v syncs" as we don't (necessarily) have the polarities set in the mode. - Drop "drm/bridge: tc358768: Add DRM_BRIDGE_ATTACH_NO_CONNECTOR support" as it's not needed for DRM_BRIDGE_ATTACH_NO_CONNECTOR support. - Link to v1: https://lore.kernel.org/r/20230804-tc358768-v1-0-1afd44b78...@ideasonboard.com Looks like I forgot to add the reviewed-bys from Peter. Sorry about that! Will add to v3. Tomi
Re: [PATCH v3 0/5] Add JDI LPM102A188A display panel support
On 16/08/2023 18:30, Thierry Reding wrote: On Mon, Aug 07, 2023 at 02:33:00PM +0100, Diogo Ivo wrote: Hello, These patches add support for the JDI LPM102A188A display panel, found in the Google Pixel C. Patch 1 adds the DT bindings for the panel. Patch 2 adds the panel driver, which is based on the downstream kernel driver published by Google and developed by Sean Paul. Patches 3-5 add DT nodes for the regulator, backlight controller and display panel. The first version of this patch series can be found at: https://lore.kernel.org/all/20220929170502.1034040-1-diogo@tecnico.ulisboa.pt/ The first submission of v2 can be found at: https://lore.kernel.org/all/20221025153746.101278-1-diogo@tecnico.ulisboa.pt/ Changes in v2: - Patch 1: remove touchscreen reset gpio property - Patch 2: clear register based on its value rather than a DT property - Patch 3: tune backlight delay values - Patch 4: add generic node names, remove underscores Changes in v3: - Patch 1: add Reviewed-by - Patch 2: fix error handling, remove enabled/prepared booleans, add dc/dc setting - Patches 3-5: Split previous patch 3 into three different patches, each adding a separate node - removed previous patch 2 pertaining to Tegra DSI reset as it was upstreamed Diogo Ivo (5): dt-bindings: display: Add bindings for JDI LPM102A188A drm/panel: Add driver for JDI LPM102A188A arm64: dts: smaug: Add DSI/CSI regulator arm64: dts: smaug: Add backlight node arm64: dts: smaug: Add display panel node I've picked up patches 3-5 into the Tegra tree and I assume the other two will go in through drm-misc? Sure, done ! Thierry
Re: (subset) [PATCH v3 0/5] Add JDI LPM102A188A display panel support
Hi, On Mon, 07 Aug 2023 14:33:00 +0100, Diogo Ivo wrote: > These patches add support for the JDI LPM102A188A display panel, > found in the Google Pixel C. > > Patch 1 adds the DT bindings for the panel. > > Patch 2 adds the panel driver, which is based on the downstream > kernel driver published by Google and developed by Sean Paul. > > [...] Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-next) [1/5] dt-bindings: display: Add bindings for JDI LPM102A188A https://cgit.freedesktop.org/drm/drm-misc/commit/?id=a913a739ab6e6ef10c0c47cb85dd4a105b3d9df7 [2/5] drm/panel: Add driver for JDI LPM102A188A https://cgit.freedesktop.org/drm/drm-misc/commit/?id=25205087df1ffe06ccea9302944ed1f77dc68c6f -- Neil
Re: [PATCH v2] drm/panel: simple: Fix Innolux G156HCE-L01 LVDS clock
Hi, On Mon, 14 Aug 2023 15:40:24 +0200, Luca Ceresoli wrote: > This panel has been implemented in commit 225213f24c79 ("drm/panel-simple: > Add Innolux G156HCE-L01 panel entry") with a higher clock than the typical > one mentioned on the documentation to avoid flickering on the unit > tested. Testing on a different unit shows that the panel actually works > with the intended 70.93 MHz clock and even lower frequencies so the > flickering is likely caused either by a defective unit or by other > different components such as the bridge. > > [...] Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-next-fixes) [1/1] drm/panel: simple: Fix Innolux G156HCE-L01 LVDS clock https://cgit.freedesktop.org/drm/drm-misc/commit/?id=438cf3271ca116253cffb8edce8aba0191327682 -- Neil
Re: [PATCH v2] drm/syncobj: fix DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE
That's actually a bit of a tricky question. It seems that the existing IGT syncobj_timeline test asserts the incorrect behavior that waiting for an unsubmitted fence with WAIT_AVAILABLE set should fail with EINVAL. I've sent a patch to igt-dev correcting this https://lists.freedesktop.org/archives/igt-dev/2023-August/059858.html however that will cause the test to fail with current kernels. I don't know how big of a problem that would be. Personally I kinda feel like the test *should* fail with current kernels, since current kernels do indeed have a bug. On 8/16/23 09:40, Simon Ser wrote: > BTW, question: do you know if we could add an IGT test to make sure we > don't regress this?
Re: [PATCH v3 2/5] drm/panel: Add driver for JDI LPM102A188A
On 07/08/2023 15:33, Diogo Ivo wrote: The JDI LPM102A188A is a 2560x1800 IPS panel found in the Google Pixel C. This driver is based on the downstream GPLv2 driver released by Google written by Sean Paul [1], which was then adapted to the newer kernel APIs. [1]: https://android.googlesource.com/kernel/tegra/+/refs/heads/android-tegra-dragon-3.18-oreo/drivers/gpu/drm/panel/panel-jdi-lpm102a188a.c Signed-off-by: Diogo Ivo --- Changes in v2: - tuned backlight delays Changed in v3: - removed "-dsi" from driver name, renamed "control"->"command" (Rayyan Ansari) - fix error handling - remove enabled/prepared booleans - add dc/dc setting function drivers/gpu/drm/panel/Kconfig | 11 + drivers/gpu/drm/panel/Makefile| 1 + drivers/gpu/drm/panel/panel-jdi-lpm102a188a.c | 551 ++ 3 files changed, 563 insertions(+) create mode 100644 drivers/gpu/drm/panel/panel-jdi-lpm102a188a.c diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 203c0ef0bbfd..787e6ac6f1e7 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -244,6 +244,17 @@ config DRM_PANEL_JDI_LT070ME05000 The panel has a 1200(RGB)×1920 (WUXGA) resolution and uses 24 bit per pixel. +config DRM_PANEL_JDI_LPM102A188A + tristate "JDI LPM102A188A DSI panel" + depends on OF && GPIOLIB + depends on DRM_MIPI_DSI + depends on BACKLIGHT_CLASS_DEVICE + help + Say Y here if you want to enable support for JDI LPM102A188A DSI + command mode panel as found in Google Pixel C devices. + The panel has a 2560×1800 resolution. It provides a MIPI DSI interface + to the host. + config DRM_PANEL_JDI_R63452 tristate "JDI R63452 Full HD DSI panel" depends on OF diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile index 30cf553c8d1d..e5a235017c86 100644 --- a/drivers/gpu/drm/panel/Makefile +++ b/drivers/gpu/drm/panel/Makefile @@ -22,6 +22,7 @@ obj-$(CONFIG_DRM_PANEL_INNOLUX_EJ030NA) += panel-innolux-ej030na.o obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o obj-$(CONFIG_DRM_PANEL_JADARD_JD9365DA_H3) += panel-jadard-jd9365da-h3.o obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o +obj-$(CONFIG_DRM_PANEL_JDI_LPM102A188A) += panel-jdi-lpm102a188a.o obj-$(CONFIG_DRM_PANEL_JDI_R63452) += panel-jdi-fhd-r63452.o obj-$(CONFIG_DRM_PANEL_KHADAS_TS050) += panel-khadas-ts050.o obj-$(CONFIG_DRM_PANEL_KINGDISPLAY_KD097D04) += panel-kingdisplay-kd097d04.o diff --git a/drivers/gpu/drm/panel/panel-jdi-lpm102a188a.c b/drivers/gpu/drm/panel/panel-jdi-lpm102a188a.c new file mode 100644 index ..5b5082efb282 --- /dev/null +++ b/drivers/gpu/drm/panel/panel-jdi-lpm102a188a.c @@ -0,0 +1,551 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2014 Google, Inc. + * + * Copyright (C) 2022 Diogo Ivo + * + * Adapted from the downstream Pixel C driver written by Sean Paul + */ + +#include +#include +#include +#include +#include +#include + +#include + +#include +#include +#include + +#define MCS_CMD_ACS_PROT 0xB0 +#define MCS_CMD_ACS_PROT_OFF (0 << 0) + +#define MCS_PWR_CTRL_FUNC 0xD0 +#define MCS_PWR_CTRL_PARAM1_DEFAULT(2 << 0) +#define MCS_PWR_CTRL_PARAM1_VGH_210_DIV(1 << 4) +#define MCS_PWR_CTRL_PARAM1_VGH_240_DIV(2 << 4) +#define MCS_PWR_CTRL_PARAM1_VGH_280_DIV(3 << 4) +#define MCS_PWR_CTRL_PARAM1_VGH_330_DIV(4 << 4) +#define MCS_PWR_CTRL_PARAM1_VGH_410_DIV(5 << 4) +#define MCS_PWR_CTRL_PARAM2_DEFAULT(9 << 4) +#define MCS_PWR_CTRL_PARAM2_VGL_210_DIV(1 << 0) +#define MCS_PWR_CTRL_PARAM2_VGL_240_DIV(2 << 0) +#define MCS_PWR_CTRL_PARAM2_VGL_280_DIV(3 << 0) +#define MCS_PWR_CTRL_PARAM2_VGL_330_DIV(4 << 0) +#define MCS_PWR_CTRL_PARAM2_VGL_410_DIV(5 << 0) + +struct jdi_panel { + struct drm_panel base; + struct mipi_dsi_device *link1; + struct mipi_dsi_device *link2; + + struct regulator *supply; + struct regulator *ddi_supply; + struct backlight_device *backlight; + + struct gpio_desc *enable_gpio; + struct gpio_desc *reset_gpio; + + const struct drm_display_mode *mode; +}; + +static inline struct jdi_panel *to_panel_jdi(struct drm_panel *panel) +{ + return container_of(panel, struct jdi_panel, base); +} + +static void jdi_wait_frames(struct jdi_panel *jdi, unsigned int frames) +{ + unsigned int refresh = drm_mode_vrefresh(jdi->mode); + + if (WARN_ON(frames > refresh)) + return; + + msleep(1000 / (refresh / frames)); +} + +static int jdi_panel_disable(struct drm_panel *panel) +{ + struct jdi_panel *jdi = to_panel_jdi(panel); + +
Re: [PATCH v2] drm/syncobj: fix DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE
BTW, question: do you know if we could add an IGT test to make sure we don't regress this?
Re: [PATCH v2 1/9] drm/sched: Convert drm scheduler to use a work queue rather than kthread
On 8/16/23 16:59, Christian König wrote: Am 16.08.23 um 14:30 schrieb Danilo Krummrich: On 8/16/23 16:05, Christian König wrote: Am 16.08.23 um 13:30 schrieb Danilo Krummrich: Hi Matt, On 8/11/23 04:31, Matthew Brost wrote: In XE, the new Intel GPU driver, a choice has made to have a 1 to 1 mapping between a drm_gpu_scheduler and drm_sched_entity. At first this seems a bit odd but let us explain the reasoning below. 1. In XE the submission order from multiple drm_sched_entity is not guaranteed to be the same completion even if targeting the same hardware engine. This is because in XE we have a firmware scheduler, the GuC, which allowed to reorder, timeslice, and preempt submissions. If a using shared drm_gpu_scheduler across multiple drm_sched_entity, the TDR falls apart as the TDR expects submission order == completion order. Using a dedicated drm_gpu_scheduler per drm_sched_entity solve this problem. 2. In XE submissions are done via programming a ring buffer (circular buffer), a drm_gpu_scheduler provides a limit on number of jobs, if the limit of number jobs is set to RING_SIZE / MAX_SIZE_PER_JOB we get flow control on the ring for free. In XE, where does the limitation of MAX_SIZE_PER_JOB come from? In Nouveau we currently do have such a limitation as well, but it is derived from the RING_SIZE, hence RING_SIZE / MAX_SIZE_PER_JOB would always be 1. However, I think most jobs won't actually utilize the whole ring. Well that should probably rather be RING_SIZE / MAX_SIZE_PER_JOB = hw_submission_limit (or even hw_submission_limit - 1 when the hw can't distinct full vs empty ring buffer). Not sure if I get you right, let me try to clarify what I was trying to say: I wanted to say that in Nouveau MAX_SIZE_PER_JOB isn't really limited by anything other than the RING_SIZE and hence we'd never allow more than 1 active job. But that lets the hw run dry between submissions. That is usually a pretty horrible idea for performance. Correct, that's the reason why I said it seems to be more efficient to base ring flow control on the actual size of each incoming job rather than the maximum size of a job. However, it seems to be more efficient to base ring flow control on the actual size of each incoming job rather than the worst case, namely the maximum size of a job. That doesn't sounds like a good idea to me. See we don't limit the number of submitted jobs based on the ring size, but rather we calculate the ring size based on the number of submitted jobs. My point isn't really about whether we derive the ring size from the job limit or the other way around. It's more about the job size (or its maximum size) being arbitrary. As mentioned in my reply to Matt: "In Nouveau, userspace can submit an arbitrary amount of addresses of indirect bufferes containing the ring instructions. The ring on the kernel side takes the addresses of the indirect buffers rather than the instructions themself. Hence, technically there isn't really a limit on the amount of IBs submitted by a job except for the ring size." So, my point is that I don't really want to limit the job size artificially just to be able to fit multiple jobs into the ring even if they're submitted at their "artificial" maximum size, but rather track how much of the ring the submitted job actually occupies. In other words the hw_submission_limit defines the ring size, not the other way around. And you usually want the hw_submission_limit as low as possible for good scheduler granularity and to avoid extra overhead. I don't think you really mean "as low as possible", do you? Because one really is the minimum if you want to do work at all, but as you mentioned above a job limit of one can let the ring run dry. In the end my proposal comes down to tracking the actual size of a job rather than just assuming a pre-defined maximum job size, and hence a dynamic job limit. I don't think this would hurt the scheduler granularity. In fact, it should even contribute to the desire of not letting the ring run dry even better. Especially for sequences of small jobs, where the current implementation might wrongly assume the ring is already full although actually there would still be enough space left. Christian. Otherwise your scheduler might just overwrite the ring buffer by pushing things to fast. Christian. Given that, it seems like it would be better to let the scheduler keep track of empty ring "slots" instead, such that the scheduler can deceide whether a subsequent job will still fit on the ring and if not re-evaluate once a previous job finished. Of course each submitted job would be required to carry the number of slots it requires on the ring. What to you think of implementing this as alternative flow control mechanism? Implementation wise this could be a union with the existing hw_submission_limit. - Danilo A problem with this design is currently a
Re: [PATCH v3 0/5] Add JDI LPM102A188A display panel support
On Mon, Aug 07, 2023 at 02:33:00PM +0100, Diogo Ivo wrote: > Hello, > > These patches add support for the JDI LPM102A188A display panel, > found in the Google Pixel C. > > Patch 1 adds the DT bindings for the panel. > > Patch 2 adds the panel driver, which is based on the downstream > kernel driver published by Google and developed by Sean Paul. > > Patches 3-5 add DT nodes for the regulator, backlight controller and > display panel. > > The first version of this patch series can be found at: > https://lore.kernel.org/all/20220929170502.1034040-1-diogo@tecnico.ulisboa.pt/ > > The first submission of v2 can be found at: > https://lore.kernel.org/all/20221025153746.101278-1-diogo@tecnico.ulisboa.pt/ > > Changes in v2: > - Patch 1: remove touchscreen reset gpio property > - Patch 2: clear register based on its value rather than a DT property > - Patch 3: tune backlight delay values > - Patch 4: add generic node names, remove underscores > > Changes in v3: > - Patch 1: add Reviewed-by > - Patch 2: fix error handling, remove enabled/prepared booleans, add >dc/dc setting > - Patches 3-5: Split previous patch 3 into three different patches, >each adding a separate node > - removed previous patch 2 pertaining to Tegra DSI reset as it was upstreamed > > Diogo Ivo (5): > dt-bindings: display: Add bindings for JDI LPM102A188A > drm/panel: Add driver for JDI LPM102A188A > arm64: dts: smaug: Add DSI/CSI regulator > arm64: dts: smaug: Add backlight node > arm64: dts: smaug: Add display panel node I've picked up patches 3-5 into the Tegra tree and I assume the other two will go in through drm-misc? Thierry signature.asc Description: PGP signature
Re: (subset) [PATCH v3 0/5] Add JDI LPM102A188A display panel support
From: Thierry Reding On Mon, 07 Aug 2023 14:33:00 +0100, Diogo Ivo wrote: > These patches add support for the JDI LPM102A188A display panel, > found in the Google Pixel C. > > Patch 1 adds the DT bindings for the panel. > > Patch 2 adds the panel driver, which is based on the downstream > kernel driver published by Google and developed by Sean Paul. > > [...] Applied, thanks! [3/5] arm64: dts: smaug: Add DSI/CSI regulator (no commit info) [4/5] arm64: dts: smaug: Add backlight node (no commit info) [5/5] arm64: dts: smaug: Add display panel node (no commit info) Best regards, -- Thierry Reding
[PATCH v2] drm/syncobj: fix DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE
If DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT is invoked with the DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE flag set but no fence has yet been submitted for the given timeline point the call will fail immediately with EINVAL. This does not match the intended behavior where the call should wait until the fence has been submitted (or the timeout expires). The following small example program illustrates the issue. It should wait for 5 seconds and then print ETIME, but instead it terminates right away after printing EINVAL. #include #include #include #include #include int main(void) { int fd = open("/dev/dri/card0", O_RDWR); uint32_t syncobj; drmSyncobjCreate(fd, 0, ); struct timespec ts; clock_gettime(CLOCK_MONOTONIC, ); uint64_t point = 1; if (drmSyncobjTimelineWait(fd, , , 1, ts.tv_sec * 10 + ts.tv_nsec + 50, // 5s DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE, NULL)) { printf("drmSyncobjTimelineWait failed %d\n", errno); } } Fixes: 01d6c3578379 ("drm/syncobj: add support for timeline point wait v8") Signed-off-by: Erik Kurzinger Reviewed by: Simon Ser --- drivers/gpu/drm/drm_syncobj.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index add45001e939..a8e6b61a188c 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -1087,7 +1087,8 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs, fence = drm_syncobj_fence_get(syncobjs[i]); if (!fence || dma_fence_chain_find_seqno(, points[i])) { dma_fence_put(fence); - if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) { + if (flags & (DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT | +DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE)) { continue; } else { timeout = -EINVAL; -- 2.41.0
[Bug 201847] nouveau 0000:01:00.0: fifo: fault 01 [WRITE] at 000000000a721000 engine 00 [GR] client 0f [GPC0/PROP_0] reason 82 [] on channel 4 [00ff85c000 X[3819]]
https://bugzilla.kernel.org/show_bug.cgi?id=201847 Simon Fogliato (simonfogli...@gmail.com) changed: What|Removed |Added CC||simonfogli...@gmail.com --- Comment #4 from Simon Fogliato (simonfogli...@gmail.com) --- sf@sf-T3600 ~ % uname -a Linux sf-T3600 6.4.10-arch1-1 #1 SMP PREEMPT_DYNAMIC Fri, 11 Aug 2023 11:03:36 + x86_64 GNU/Linux Aug 16 08:21:06 sf-T3600 kernel: nouveau :03:00.0: fifo: fault 01 [WRITE] at 0002e000 engine 15 [PCE0] client 01 [HUB/PCOPY0] reason 02 [PAGE_NOT_PRES> Aug 16 08:21:06 sf-T3600 kernel: nouveau :03:00.0: fifo:00:0001:[(udev-worker)[738]] rc scheduled Aug 16 08:21:06 sf-T3600 kernel: nouveau :03:00.0: fifo:00: rc scheduled Aug 16 08:21:06 sf-T3600 kernel: nouveau :03:00.0: fifo:00:0001:0001:[(udev-worker)[738]] errored - disabling channel Aug 16 08:21:06 sf-T3600 kernel: nouveau :03:00.0: DRM: channel 1 killed! Aug 16 08:21:08 sf-T3600 kernel: sched: RT throttling activated Aug 16 08:21:44 sf-T3600 rtkit-daemon[1065]: Supervising 8 threads of 5 processes of 1 users. Aug 16 08:21:44 sf-T3600 rtkit-daemon[1065]: Supervising 8 threads of 5 processes of 1 users. Aug 16 08:21:49 sf-T3600 kernel: [ cut here ] Aug 16 08:21:49 sf-T3600 kernel: WARNING: CPU: 0 PID: 19149 at mm/gup.c:1101 __get_user_pages+0x582/0x680 Aug 16 08:21:49 sf-T3600 kernel: Modules linked in: snd_seq_dummy snd_hrtimer snd_seq rfkill intel_rapl_msr intel_rapl_common uvcvideo x86_pkg_temp_thermal intel_> Aug 16 08:21:49 sf-T3600 kernel: crct10dif_pclmul crc32_pclmul crc32c_intel polyval_clmulni uas polyval_generic usb_storage usbhid gf128mul ghash_clmulni_intel s> Aug 16 08:21:49 sf-T3600 kernel: CPU: 0 PID: 19149 Comm: chrome_crashpad Tainted: GW OE 6.4.10-arch1-1 #1 2d4402bf7ad4a7ea488c9261840b8101c9d1e712 Aug 16 08:21:49 sf-T3600 kernel: Hardware name: Dell Inc. Precision T3600/08HPGT, BIOS A15 05/08/2017 Aug 16 08:21:49 sf-T3600 kernel: RIP: 0010:__get_user_pages+0x582/0x680 Aug 16 08:21:49 sf-T3600 kernel: Code: 00 e9 cb fd ff ff 48 03 bd 88 00 00 00 e9 c7 fb ff ff 48 81 e1 00 f0 ff ff e9 4b fc ff ff 48 81 e2 00 f0 ff ff e9 b5 fc ff > Aug 16 08:21:49 sf-T3600 kernel: RSP: 0018:b531ccc17bf8 EFLAGS: 00010202 Aug 16 08:21:49 sf-T3600 kernel: RAX: 94633a009cc0 RBX: 0005000a RCX: 7ffdc4e02fff Aug 16 08:21:49 sf-T3600 kernel: RDX: RSI: 7eff84e4b000 RDI: 9463c986a080 Aug 16 08:21:49 sf-T3600 kernel: RBP: 946398f0bc80 R08: 94633a0b8008 R09: 0001 Aug 16 08:21:49 sf-T3600 kernel: R10: 94633a0b8080 R11: 94633a0b800c R12: Aug 16 08:21:49 sf-T3600 kernel: R13: 94633a009cc0 R14: b531ccc17cbc R15: b531ccc17cbc Aug 16 08:21:49 sf-T3600 kernel: FS: 7f7035d135c0() GS:946a2f60() knlGS: Aug 16 08:21:49 sf-T3600 kernel: CS: 0010 DS: ES: CR0: 80050033 Aug 16 08:21:49 sf-T3600 kernel: CR2: 36cc0040c300 CR3: 00018e95e004 CR4: 000626f0 Aug 16 08:21:49 sf-T3600 kernel: Call Trace: Aug 16 08:21:49 sf-T3600 kernel: Aug 16 08:21:49 sf-T3600 kernel: ? __get_user_pages+0x582/0x680 Aug 16 08:21:49 sf-T3600 kernel: ? __warn+0x81/0x130 Aug 16 08:21:49 sf-T3600 kernel: ? __get_user_pages+0x582/0x680 Aug 16 08:21:49 sf-T3600 kernel: ? report_bug+0x171/0x1a0 Aug 16 08:21:49 sf-T3600 kernel: ? handle_bug+0x3c/0x80 Aug 16 08:21:49 sf-T3600 kernel: ? exc_invalid_op+0x17/0x70 Aug 16 08:21:49 sf-T3600 kernel: ? asm_exc_invalid_op+0x1a/0x20 Aug 16 08:21:49 sf-T3600 kernel: ? __get_user_pages+0x582/0x680 Aug 16 08:21:49 sf-T3600 kernel: ? __get_user_pages+0x8a/0x680 Aug 16 08:21:49 sf-T3600 kernel: get_user_pages_remote+0x14a/0x400 Aug 16 08:21:49 sf-T3600 kernel: __access_remote_vm+0x1bf/0x420 Aug 16 08:21:49 sf-T3600 kernel: mem_rw.isra.0+0x111/0x1d0 Aug 16 08:21:49 sf-T3600 kernel: vfs_read+0xac/0x320 Aug 16 08:21:49 sf-T3600 kernel: ? mem_rw.isra.0+0x18a/0x1d0 Aug 16 08:21:49 sf-T3600 kernel: ? vfs_read+0xac/0x320 Aug 16 08:21:49 sf-T3600 kernel: __x64_sys_pread64+0x98/0xd0 Aug 16 08:21:49 sf-T3600 kernel: do_syscall_64+0x60/0x90 Aug 16 08:21:49 sf-T3600 kernel: ? __x64_sys_pread64+0xa8/0xd0 Aug 16 08:21:49 sf-T3600 kernel: ? syscall_exit_to_user_mode+0x1b/0x40 Aug 16 08:21:49 sf-T3600 kernel: ? do_syscall_64+0x6c/0x90 Aug 16 08:21:49 sf-T3600 kernel: ? do_syscall_64+0x6c/0x90 Aug 16 08:21:49 sf-T3600 kernel: entry_SYSCALL_64_after_hwframe+0x77/0xe1 Aug 16 08:21:49 sf-T3600 kernel: RIP: 0033:0x7f7035ae8d07 Aug 16 08:21:49 sf-T3600 kernel: Code: 08 89 3c 24 48 89 4c 24 18 e8 85 00 fa ff 4c 8b 54 24 18 48 8b 54 24 10 41 89 c0 48 8b 74 24 08 8b 3c 24 b8 11 00 00 00 0f > Aug 16 08:21:49 sf-T3600 kernel: RSP: 002b:7fff5f79a8f0 EFLAGS: 0293 ORIG_RAX: 0011 Aug 16 08:21:49 sf-T3600 kernel: RAX:
Re: [PATCH v2 09/15] drm/panthor: Add the FW logical block
On 09/08/2023 17:53, Boris Brezillon wrote: > Contains everything that's FW related, that includes the code dealing > with the microcontroller unit (MCU) that's running the FW, and anything > related to allocating memory shared between the FW and the CPU. > > A few global FW events are processed in the IRQ handler, the rest is > forwarded to the scheduler, since scheduling is the primary reason for > the FW existence, and also the main source of FW <-> kernel > interactions. > > v2: > - Rename the driver (pancsf -> panthor) > - Rename the file (_mcu -> _fw) > - Change the license (GPL2 -> MIT + GPL2) > - Split the driver addition commit > - Document the code > - Use drm_dev_{unplug,enter,exit}() to provide safe device removal > - Use the panthor_irq layer to manage/process IRQs > > Signed-off-by: Boris Brezillon > --- > drivers/gpu/drm/panthor/panthor_fw.c | 1417 ++ > drivers/gpu/drm/panthor/panthor_fw.h | 505 + > 2 files changed, 1922 insertions(+) > create mode 100644 drivers/gpu/drm/panthor/panthor_fw.c > create mode 100644 drivers/gpu/drm/panthor/panthor_fw.h > > diff --git a/drivers/gpu/drm/panthor/panthor_fw.c > b/drivers/gpu/drm/panthor/panthor_fw.c > new file mode 100644 > index ..359a68f7af03 > --- /dev/null > +++ b/drivers/gpu/drm/panthor/panthor_fw.c > @@ -0,0 +1,1417 @@ > +// SPDX-License-Identifier: GPL-2.0 or MIT > +/* Copyright 2023 Collabora ltd. */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > + > +#include "panthor_device.h" > +#include "panthor_gem.h" > +#include "panthor_gpu.h" > +#include "panthor_regs.h" > +#include "panthor_fw.h" > +#include "panthor_mmu.h" > +#include "panthor_sched.h" > + > +#define CSF_FW_NAME "mali_csffw.bin" > + > +#define PING_INTERVAL_MS 12000 > +#define PROGRESS_TIMEOUT_CYCLES (5ull * 500 * 1024 * > 1024) > +#define PROGRESS_TIMEOUT_SCALE_SHIFT 10 > +#define IDLE_HYSTERESIS_US 800 > +#define PWROFF_HYSTERESIS_US 1 > + > +/** > + * struct panthor_fw_mem - FW memory > + */ > +struct panthor_fw_mem { > + /** @bo: Buffer object backing the FW memory. */ > + struct panthor_gem_object *bo; > + > + /** @kmap: Kernel CPU mapping of the FW memory. */ > + void *kmap; > + > + /** @va: MCU mapping of the FW memory. */ > + u64 va; > +}; > + > +/** > + * struct panthor_fw_binary_hdr - Firmware binary header. > + */ > +struct panthor_fw_binary_hdr { > + /** @magic: Magic value to check binary validity. */ > + u32 magic; > +#define CSF_FW_BINARY_HEADER_MAGIC 0xc3f13a6e > + > + /** @minor: Minor FW version. */ > + u8 minor; > + > + /** @major: Major FW version. */ > + u8 major; > +#define CSF_FW_BINARY_HEADER_MAJOR_MAX 0 > + > + /** @padding1: MBZ. */ > + u16 padding1; > + > + /** @version_hash: FW version hash. */ > + u32 version_hash; > + > + /** @padding2: MBZ. */ > + u32 padding2; > + > + /** @size: FW binary size. */ > + u32 size; > +}; > + > +/** > + * enum panthor_fw_binary_entry_type - Firmware binary entry type > + */ > +enum panthor_fw_binary_entry_type { > + /** @CSF_FW_BINARY_ENTRY_TYPE_IFACE: Host <-> FW interface. */ > + CSF_FW_BINARY_ENTRY_TYPE_IFACE = 0, > + > + /** @CSF_FW_BINARY_ENTRY_TYPE_CONFIG: FW config. */ > + CSF_FW_BINARY_ENTRY_TYPE_CONFIG = 1, > + > + /** @CSF_FW_BINARY_ENTRY_TYPE_FUTF_TEST: Unit-tests. */ > + CSF_FW_BINARY_ENTRY_TYPE_FUTF_TEST = 2, > + > + /** @CSF_FW_BINARY_ENTRY_TYPE_TRACE_BUFFER: Trace buffer interface. */ > + CSF_FW_BINARY_ENTRY_TYPE_TRACE_BUFFER = 3, > + > + /** @CSF_FW_BINARY_ENTRY_TYPE_TIMELINE_METADATA: Timeline metadata > interface. */ > + CSF_FW_BINARY_ENTRY_TYPE_TIMELINE_METADATA = 4, > +}; > + > +#define CSF_FW_BINARY_ENTRY_TYPE(ehdr) > ((ehdr) & 0xff) > +#define CSF_FW_BINARY_ENTRY_SIZE(ehdr) > (((ehdr) >> 8) & 0xff) > +#define CSF_FW_BINARY_ENTRY_UPDATE BIT(30) > +#define CSF_FW_BINARY_ENTRY_OPTIONAL BIT(31) > + > +#define CSF_FW_BINARY_IFACE_ENTRY_RD_RD > BIT(0) > +#define CSF_FW_BINARY_IFACE_ENTRY_RD_WR > BIT(1) > +#define CSF_FW_BINARY_IFACE_ENTRY_RD_EX > BIT(2) > +#define CSF_FW_BINARY_IFACE_ENTRY_RD_CACHE_MODE_NONE (0 << 3) > +#define CSF_FW_BINARY_IFACE_ENTRY_RD_CACHE_MODE_CACHED > (1 << 3) > +#define CSF_FW_BINARY_IFACE_ENTRY_RD_CACHE_MODE_UNCACHED_COHERENT(2 << 3) > +#define CSF_FW_BINARY_IFACE_ENTRY_RD_CACHE_MODE_CACHED_COHERENT > (3 << 3) > +#define CSF_FW_BINARY_IFACE_ENTRY_RD_CACHE_MODE_MASK > GENMASK(4, 3) > +#define
RE: [PATCH -next] drm/amd/display: Simplify bool conversion
[AMD Official Use Only - General] Thanks Li, for the fix, the fix is already in process of merging into amd-staging-drm-next. https://patchwork.freedesktop.org/patch/552568/ -Original Message- From: amd-gfx On Behalf Of Yang Li Sent: Wednesday, August 16, 2023 6:16 AM To: airl...@gmail.com; dan...@ffwll.ch; Deucher, Alexander ; Wentland, Harry ; Siqueira, Rodrigo Cc: Yang Li ; dri-devel@lists.freedesktop.org; amd-...@lists.freedesktop.org; linux-ker...@vger.kernel.org Subject: [PATCH -next] drm/amd/display: Simplify bool conversion ./drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_replay.c:94:102-107: WARNING: conversion to bool not needed here ./drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_replay.c:102:72-77: WARNING: conversion to bool not needed here Signed-off-by: Yang Li --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_replay.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_replay.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_replay.c index 32d3086c4cb7..5ce542b1f860 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_replay.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_replay.c @@ -91,7 +91,7 @@ bool amdgpu_dm_setup_replay(struct dc_link *link, struct amdgpu_dm_connector *ac pr_config.replay_supported = true; pr_config.replay_power_opt_supported = 0; pr_config.replay_enable_option |= pr_enable_option_static_screen; - pr_config.replay_timing_sync_supported = aconnector->max_vfreq >= 2 * aconnector->min_vfreq ? true : false; + pr_config.replay_timing_sync_supported = aconnector->max_vfreq >= 2 * +aconnector->min_vfreq; if (!pr_config.replay_timing_sync_supported) pr_config.replay_enable_option &= ~pr_enable_option_general_ui; @@ -99,7 +99,7 @@ bool amdgpu_dm_setup_replay(struct dc_link *link, struct amdgpu_dm_connector *ac debug_flags = (union replay_debug_flags *)_config.debug_flags; debug_flags->u32All = 0; debug_flags->bitfields.visual_confirm = - link->ctx->dc->debug.visual_confirm == VISUAL_CONFIRM_REPLAY ? true : false; + link->ctx->dc->debug.visual_confirm == VISUAL_CONFIRM_REPLAY; link->replay_settings.replay_feature_enabled = true; -- 2.20.1.7.g153144c
Re: [PATCH v2 1/9] drm/sched: Convert drm scheduler to use a work queue rather than kthread
On 8/16/23 16:38, Matthew Brost wrote: On Wed, Aug 16, 2023 at 02:30:38PM +0200, Danilo Krummrich wrote: On 8/16/23 16:05, Christian König wrote: Am 16.08.23 um 13:30 schrieb Danilo Krummrich: Hi Matt, On 8/11/23 04:31, Matthew Brost wrote: In XE, the new Intel GPU driver, a choice has made to have a 1 to 1 mapping between a drm_gpu_scheduler and drm_sched_entity. At first this seems a bit odd but let us explain the reasoning below. 1. In XE the submission order from multiple drm_sched_entity is not guaranteed to be the same completion even if targeting the same hardware engine. This is because in XE we have a firmware scheduler, the GuC, which allowed to reorder, timeslice, and preempt submissions. If a using shared drm_gpu_scheduler across multiple drm_sched_entity, the TDR falls apart as the TDR expects submission order == completion order. Using a dedicated drm_gpu_scheduler per drm_sched_entity solve this problem. 2. In XE submissions are done via programming a ring buffer (circular buffer), a drm_gpu_scheduler provides a limit on number of jobs, if the limit of number jobs is set to RING_SIZE / MAX_SIZE_PER_JOB we get flow control on the ring for free. In XE, where does the limitation of MAX_SIZE_PER_JOB come from? In Xe the job submission is series of ring instructions done by the KMD. The instructions are cache flushes, seqno writes, jump to user BB, etc... The exact instructions for each job vary based on hw engine type, platform, etc... We dervive MAX_SIZE_PER_JOB from the largest set of instructions to submit a job and have a define in the driver for this. I believe it is currently set to 192 bytes (the actual define is MAX_JOB_SIZE_BYTES). So a 16k ring lets Xe have 85 jobs inflight at once. Ok, that sounds different to how Nouveau works. The "largest set of instructions to submit a job" really is a given by how the hardware works rather than an arbitrary limit? In Nouveau, userspace can submit an arbitrary amount of addresses of indirect bufferes containing the ring instructions. The ring on the kernel side takes the addresses of the indirect buffers rather than the instructions themself. Hence, technically there isn't really a limit on the amount of IBs submitted by a job except for the ring size. In Nouveau we currently do have such a limitation as well, but it is derived from the RING_SIZE, hence RING_SIZE / MAX_SIZE_PER_JOB would always be 1. However, I think most jobs won't actually utilize the whole ring. Well that should probably rather be RING_SIZE / MAX_SIZE_PER_JOB = hw_submission_limit (or even hw_submission_limit - 1 when the hw can't Yes, hw_submission_limit = RING_SIZE / MAX_SIZE_PER_JOB in Xe. distinct full vs empty ring buffer). Not sure if I get you right, let me try to clarify what I was trying to say: I wanted to say that in Nouveau MAX_SIZE_PER_JOB isn't really limited by anything other than the RING_SIZE and hence we'd never allow more than 1 active job. I'm confused how there isn't a limit on the size of the job in Nouveau? Based on what you have said, a job could be larger than the ring then? As explained above, theoretically it could. It's only limited by the ring size. However, it seems to be more efficient to base ring flow control on the actual size of each incoming job rather than the worst case, namely the maximum size of a job. If this doesn't work for Nouveau, feel free flow control the ring differently but this works rather well (and simple) for Xe. Matt Otherwise your scheduler might just overwrite the ring buffer by pushing things to fast. Christian. Given that, it seems like it would be better to let the scheduler keep track of empty ring "slots" instead, such that the scheduler can deceide whether a subsequent job will still fit on the ring and if not re-evaluate once a previous job finished. Of course each submitted job would be required to carry the number of slots it requires on the ring. What to you think of implementing this as alternative flow control mechanism? Implementation wise this could be a union with the existing hw_submission_limit. - Danilo A problem with this design is currently a drm_gpu_scheduler uses a kthread for submission / job cleanup. This doesn't scale if a large number of drm_gpu_scheduler are used. To work around the scaling issue, use a worker rather than kthread for submission / job cleanup. v2: - (Rob Clark) Fix msm build - Pass in run work queue v3: - (Boris) don't have loop in worker v4: - (Tvrtko) break out submit ready, stop, start helpers into own patch Signed-off-by: Matthew Brost
[Bug 217664] Laptop doesnt wake up from suspend mode.
https://bugzilla.kernel.org/show_bug.cgi?id=217664 --- Comment #11 from Jose Roberto (colombo...@gmail.com) --- If I may: have you tried to disable (or even better, uninstall) tlp? Most of modern GNU/Linuxes come with it previously installed. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[Bug 217664] Laptop doesnt wake up from suspend mode.
https://bugzilla.kernel.org/show_bug.cgi?id=217664 --- Comment #10 from popus_czy_to_ty (pentelja...@o2.pl) --- aafter reinstalling and many times trying response for your questions is 1 (mux) whatever it is --> no 2 no 3 no, via command its dead also 4.no 5. insyde corp (bios manufacturer doesnt respond to my emails), even in bios i cant disable dedicated card -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[GIT PULL] etnaviv-next for 6.6
Hi Dave, Daniel, please pull the following etnaviv changes for the next merge window. This time mostly cleanups around the runtime power management handling and slightly improved GPU hang handling. Also some additions to the HWDB to get the driver working properly on more NXP i.MX8MP IP cores. Sorry for being late, vacation got in the way, all changes have been soaking in linux-next for more than 3 weeks. Regards, Lucas The following changes since commit ac9a78681b921877518763ba0e89202254349d1b: Linux 6.4-rc1 (2023-05-07 13:34:35 -0700) are available in the Git repository at: https://git.pengutronix.de/git/lst/linux etnaviv/next for you to fetch changes up to 88c31d2dd191ab78e9ba9ff967845018aa7ee214: drm/etnaviv: fix error code in event_alloc() (2023-07-19 12:36:25 +0200) Dan Carpenter (1): drm/etnaviv: fix error code in event_alloc() Lucas Stach (12): drm/etnaviv: slow down FE idle polling drm/etnaviv: fix dumping of active MMU context drm/etnaviv: add HWDB entry for VIP8000 Nano r8002 drm/etnaviv: add HWDB entry for GC520 r5341 c204 drm/etnaviv: move down etnaviv_gpu_recover_hang() in file drm/etnaviv: free events the usual way in recover worker drm/etnaviv: move runtime PM handling to events drm/etnaviv: make clock handling symetric between runtime resume and suspend drm/etnaviv: avoid runtime PM usage in etnaviv_gpu_bind drm/etnaviv: better track GPU state drm/etnaviv: drop GPU initialized property drm/etnaviv: expedited MMU fault handling Rob Herring (1): drm: etnaviv: Replace of_platform.h with explicit includes drivers/gpu/drm/etnaviv/etnaviv_buffer.c | 11 +-- drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c | 3 + drivers/gpu/drm/etnaviv/etnaviv_drv.c| 4 +- drivers/gpu/drm/etnaviv/etnaviv_dump.c | 14 ++-- drivers/gpu/drm/etnaviv/etnaviv_gem.h| 1 - drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 3 - drivers/gpu/drm/etnaviv/etnaviv_gpu.c| 188 - drivers/gpu/drm/etnaviv/etnaviv_gpu.h| 13 +++- drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 63 +++ drivers/gpu/drm/etnaviv/etnaviv_mmu.c| 3 + drivers/gpu/drm/etnaviv/etnaviv_sched.c | 5 +- 11 files changed, 201 insertions(+), 107 deletions(-)
Re: [PATCH] drm/nouveau/disp: fix use-after-free in error handling of nouveau_connector_create
On Wed, Aug 16, 2023 at 04:57:28PM +0200, Karol Herbst wrote: > Do you have any connectors listed in "/sys/class/drm"? tree /sys/class/drm/ /sys/class/drm/ ├── card0 -> ../../devices/pci:00/:00:02.0/:03:00.0/drm/card0 ├── renderD128 -> ../../devices/pci:00/:00:02.0/:03:00.0/drm/renderD128 └── version > Also, mind sharing your vbios.rom file from > "/sys/kernel/debug/dri/0/vbios.rom"? Attached. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette vbios.rom.gz Description: application/gzip
Re: [PATCH 1/5] mm: move some shrinker-related function declarations to mm/internal.h
Hi Qi, kernel test robot noticed the following build warnings: [auto build test WARNING on brauner-vfs/vfs.all] [also build test WARNING on linus/master v6.5-rc6 next-20230816] [cannot apply to akpm-mm/mm-everything drm-misc/drm-misc-next vfs-idmapping/for-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Qi-Zheng/mm-move-some-shrinker-related-function-declarations-to-mm-internal-h/20230816-163833 base: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.all patch link: https://lore.kernel.org/r/20230816083419.41088-2-zhengqi.arch%40bytedance.com patch subject: [PATCH 1/5] mm: move some shrinker-related function declarations to mm/internal.h config: x86_64-buildonly-randconfig-r003-20230816 (https://download.01.org/0day-ci/archive/20230816/202308162208.cqbngoer-...@intel.com/config) compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0 reproduce: (https://download.01.org/0day-ci/archive/20230816/202308162208.cqbngoer-...@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/202308162208.cqbngoer-...@intel.com/ All warnings (new ones prefixed by >>): >> mm/shrinker_debug.c:174:5: warning: no previous declaration for >> 'shrinker_debugfs_add' [-Wmissing-declarations] int shrinker_debugfs_add(struct shrinker *shrinker) ^~~~ >> mm/shrinker_debug.c:249:16: warning: no previous declaration for >> 'shrinker_debugfs_detach' [-Wmissing-declarations] struct dentry *shrinker_debugfs_detach(struct shrinker *shrinker, ^~~ >> mm/shrinker_debug.c:265:6: warning: no previous declaration for >> 'shrinker_debugfs_remove' [-Wmissing-declarations] void shrinker_debugfs_remove(struct dentry *debugfs_entry, int debugfs_id) ^~~ vim +/shrinker_debugfs_add +174 mm/shrinker_debug.c bbf535fd6f06b9 Roman Gushchin 2022-05-31 173 5035ebc644aec9 Roman Gushchin 2022-05-31 @174 int shrinker_debugfs_add(struct shrinker *shrinker) 5035ebc644aec9 Roman Gushchin 2022-05-31 175 { 5035ebc644aec9 Roman Gushchin 2022-05-31 176 struct dentry *entry; e33c267ab70de4 Roman Gushchin 2022-05-31 177 char buf[128]; 5035ebc644aec9 Roman Gushchin 2022-05-31 178 int id; 5035ebc644aec9 Roman Gushchin 2022-05-31 179 47a7c01c3efc65 Qi Zheng 2023-06-09 180 lockdep_assert_held(_rwsem); 5035ebc644aec9 Roman Gushchin 2022-05-31 181 5035ebc644aec9 Roman Gushchin 2022-05-31 182 /* debugfs isn't initialized yet, add debugfs entries later. */ 5035ebc644aec9 Roman Gushchin 2022-05-31 183 if (!shrinker_debugfs_root) 5035ebc644aec9 Roman Gushchin 2022-05-31 184 return 0; 5035ebc644aec9 Roman Gushchin 2022-05-31 185 5035ebc644aec9 Roman Gushchin 2022-05-31 186 id = ida_alloc(_debugfs_ida, GFP_KERNEL); 5035ebc644aec9 Roman Gushchin 2022-05-31 187 if (id < 0) 5035ebc644aec9 Roman Gushchin 2022-05-31 188 return id; 5035ebc644aec9 Roman Gushchin 2022-05-31 189 shrinker->debugfs_id = id; 5035ebc644aec9 Roman Gushchin 2022-05-31 190 e33c267ab70de4 Roman Gushchin 2022-05-31 191 snprintf(buf, sizeof(buf), "%s-%d", shrinker->name, id); 5035ebc644aec9 Roman Gushchin 2022-05-31 192 5035ebc644aec9 Roman Gushchin 2022-05-31 193 /* create debugfs entry */ 5035ebc644aec9 Roman Gushchin 2022-05-31 194 entry = debugfs_create_dir(buf, shrinker_debugfs_root); 5035ebc644aec9 Roman Gushchin 2022-05-31 195 if (IS_ERR(entry)) { 5035ebc644aec9 Roman Gushchin 2022-05-31 196 ida_free(_debugfs_ida, id); 5035ebc644aec9 Roman Gushchin 2022-05-31 197 return PTR_ERR(entry); 5035ebc644aec9 Roman Gushchin 2022-05-31 198 } 5035ebc644aec9 Roman Gushchin 2022-05-31 199 shrinker->debugfs_entry = entry; 5035ebc644aec9 Roman Gushchin 2022-05-31 200 2124f79de6a909 John Keeping 2023-04-18 201 debugfs_create_file("count", 0440, entry, shrinker, 5035ebc644aec9 Roman Gushchin 2022-05-31 202 _debugfs_count_fops); 2124f79de6a909 John Keeping 2023-04-18 203 debugfs_create_file("scan", 0220, entry, shrinker, bbf535fd6f06b9 Roman Gushchin 2022-05-31 204 _debugfs_scan_fops); 5035ebc644aec9 Roman Gushchin 2022-05-31 205 return 0; 5035ebc644aec9 Roman Gushchin 2022-05-31 206 } 5035ebc644aec9 Roman G
Re: [PATCH v2 1/9] drm/sched: Convert drm scheduler to use a work queue rather than kthread
Am 16.08.23 um 14:30 schrieb Danilo Krummrich: On 8/16/23 16:05, Christian König wrote: Am 16.08.23 um 13:30 schrieb Danilo Krummrich: Hi Matt, On 8/11/23 04:31, Matthew Brost wrote: In XE, the new Intel GPU driver, a choice has made to have a 1 to 1 mapping between a drm_gpu_scheduler and drm_sched_entity. At first this seems a bit odd but let us explain the reasoning below. 1. In XE the submission order from multiple drm_sched_entity is not guaranteed to be the same completion even if targeting the same hardware engine. This is because in XE we have a firmware scheduler, the GuC, which allowed to reorder, timeslice, and preempt submissions. If a using shared drm_gpu_scheduler across multiple drm_sched_entity, the TDR falls apart as the TDR expects submission order == completion order. Using a dedicated drm_gpu_scheduler per drm_sched_entity solve this problem. 2. In XE submissions are done via programming a ring buffer (circular buffer), a drm_gpu_scheduler provides a limit on number of jobs, if the limit of number jobs is set to RING_SIZE / MAX_SIZE_PER_JOB we get flow control on the ring for free. In XE, where does the limitation of MAX_SIZE_PER_JOB come from? In Nouveau we currently do have such a limitation as well, but it is derived from the RING_SIZE, hence RING_SIZE / MAX_SIZE_PER_JOB would always be 1. However, I think most jobs won't actually utilize the whole ring. Well that should probably rather be RING_SIZE / MAX_SIZE_PER_JOB = hw_submission_limit (or even hw_submission_limit - 1 when the hw can't distinct full vs empty ring buffer). Not sure if I get you right, let me try to clarify what I was trying to say: I wanted to say that in Nouveau MAX_SIZE_PER_JOB isn't really limited by anything other than the RING_SIZE and hence we'd never allow more than 1 active job. But that lets the hw run dry between submissions. That is usually a pretty horrible idea for performance. However, it seems to be more efficient to base ring flow control on the actual size of each incoming job rather than the worst case, namely the maximum size of a job. That doesn't sounds like a good idea to me. See we don't limit the number of submitted jobs based on the ring size, but rather we calculate the ring size based on the number of submitted jobs. In other words the hw_submission_limit defines the ring size, not the other way around. And you usually want the hw_submission_limit as low as possible for good scheduler granularity and to avoid extra overhead. Christian. Otherwise your scheduler might just overwrite the ring buffer by pushing things to fast. Christian. Given that, it seems like it would be better to let the scheduler keep track of empty ring "slots" instead, such that the scheduler can deceide whether a subsequent job will still fit on the ring and if not re-evaluate once a previous job finished. Of course each submitted job would be required to carry the number of slots it requires on the ring. What to you think of implementing this as alternative flow control mechanism? Implementation wise this could be a union with the existing hw_submission_limit. - Danilo A problem with this design is currently a drm_gpu_scheduler uses a kthread for submission / job cleanup. This doesn't scale if a large number of drm_gpu_scheduler are used. To work around the scaling issue, use a worker rather than kthread for submission / job cleanup. v2: - (Rob Clark) Fix msm build - Pass in run work queue v3: - (Boris) don't have loop in worker v4: - (Tvrtko) break out submit ready, stop, start helpers into own patch Signed-off-by: Matthew Brost
Re: [PATCH] drm/nouveau/disp: fix use-after-free in error handling of nouveau_connector_create
On Wed, Aug 16, 2023 at 4:54 PM Borislav Petkov wrote: > > On Wed, Aug 16, 2023 at 11:51:50AM +0200, Karol Herbst wrote: > > Mind sharing your kernel logs with that patch applied? I suspect your > > system boots up but you might just not have the connector available or > > something? It could be that you have one of those GPUs affected by the > > original change and then we'd have to figure out what to do with that. > > Close. With your patch applied, the machine is up and I can log in and > use it. However, the output on the connected monitor stops after... > > [6.815167] ACPI: \_PR_.CP05: Found 4 idle states > [6.825438] ACPI: \_PR_.CP06: Found 4 idle states > [6.835661] ACPI: \_PR_.CP07: Found 4 idle states > [7.280093] Freeing initrd memory: 8328K > [7.601986] tsc: Refined TSC clocksource calibration: 3591.346 MHz > [7.608360] clocksource: tsc: mask: 0x max_cycles: > 0x33c46403b59, max_idle_ns: 440795293818 ns > [7.620254] clocksource: Switched to clocksource tsc > [8.337724] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled > [8.350553] 00:05: ttyS0 at I/O 0x3f8 (irq = 4, base_baud = 115200) is a > 16550A > [8.375311] serial :00:16.3: enabling device ( -> 0003) > [8.403681] :00:16.3: ttyS1 at I/O 0xf0a0 (irq = 17, base_baud = > 115200) is a 16550A > [8.424951] Linux agpgart interface v0.103 > [8.432456] ACPI: bus type drm_connector registered > > ... this line here above. It is the last one output. What you see here > below what I'm catching from serial. > > [8.456734] Console: switching to colour dummy device 80x25 > [8.464414] nouveau :03:00.0: vgaarb: deactivate vga console > [8.473063] nouveau :03:00.0: NVIDIA GT218 (0a8c00b1) > [8.594096] nouveau :03:00.0: bios: version 70.18.83.00.08 > [8.607906] nouveau :03:00.0: fb: 512 MiB DDR3 > [8.926721] nouveau :03:00.0: DRM: VRAM: 512 MiB > [8.931763] nouveau :03:00.0: DRM: GART: 1048576 MiB > [8.937156] nouveau :03:00.0: DRM: TMDS table version 2.0 > [8.942969] nouveau :03:00.0: DRM: DCB version 4.0 > [8.948173] nouveau :03:00.0: DRM: DCB outp 00: 02000360 > [8.954696] nouveau :03:00.0: DRM: DCB outp 01: 02000362 00020010 > [8.961211] nouveau :03:00.0: DRM: DCB outp 02: 028003a6 0f220010 > [8.967739] nouveau :03:00.0: DRM: DCB outp 03: 01011380 > [8.974261] nouveau :03:00.0: DRM: DCB outp 04: 08011382 00020010 > [8.980769] nouveau :03:00.0: DRM: DCB outp 05: 088113c6 0f220010 > [8.987293] nouveau :03:00.0: DRM: DCB conn 00: 00101064 > [8.993015] nouveau :03:00.0: DRM: DCB conn 01: 00202165 > [9.005724] nouveau :03:00.0: DRM: MM: using COPY for buffer copies > [9.023889] [drm] Initialized nouveau 1.3.1 20120801 for :03:00.0 on > minor 0 > [9.032044] nouveau :03:00.0: [drm] Cannot find any crtc or sizes > [9.162909] megasas: 07.725.01.00-rc1 > [9.167537] st: Version 20160209, fixed bufsize 32768, s/g segs 256 > [9.176058] ahci :00:1f.2: version 3.0 > [9.194078] ahci :00:1f.2: AHCI 0001.0300 32 slots 6 ports 6 Gbps 0x3 > impl SATA mode > [9.202487] ahci :00:1f.2: flags: 64bit ncq sntf pm led clo pio slum > part ems apst > [9.243154] scsi host0: ahci > [9.252090] scsi host1: ahci > [9.260389] scsi host2: ahci > [9.268061] scsi host3: ahci > [9.273542] scsi host4: ahci > [9.279071] scsi host5: ahci > ... > > and so on until full boot. > okay, so the patch at least fixes the memory corruption, which is good, so I'd go ahead and push it out as it might also fix other unrelated crashes. Do you have any connectors listed in "/sys/class/drm"? Also, mind sharing your vbios.rom file from "/sys/kernel/debug/dri/0/vbios.rom"? Thanks > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette >
Re: [PATCH] drm/nouveau/disp: fix use-after-free in error handling of nouveau_connector_create
On Wed, Aug 16, 2023 at 11:51:50AM +0200, Karol Herbst wrote: > Mind sharing your kernel logs with that patch applied? I suspect your > system boots up but you might just not have the connector available or > something? It could be that you have one of those GPUs affected by the > original change and then we'd have to figure out what to do with that. Close. With your patch applied, the machine is up and I can log in and use it. However, the output on the connected monitor stops after... [6.815167] ACPI: \_PR_.CP05: Found 4 idle states [6.825438] ACPI: \_PR_.CP06: Found 4 idle states [6.835661] ACPI: \_PR_.CP07: Found 4 idle states [7.280093] Freeing initrd memory: 8328K [7.601986] tsc: Refined TSC clocksource calibration: 3591.346 MHz [7.608360] clocksource: tsc: mask: 0x max_cycles: 0x33c46403b59, max_idle_ns: 440795293818 ns [7.620254] clocksource: Switched to clocksource tsc [8.337724] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled [8.350553] 00:05: ttyS0 at I/O 0x3f8 (irq = 4, base_baud = 115200) is a 16550A [8.375311] serial :00:16.3: enabling device ( -> 0003) [8.403681] :00:16.3: ttyS1 at I/O 0xf0a0 (irq = 17, base_baud = 115200) is a 16550A [8.424951] Linux agpgart interface v0.103 [8.432456] ACPI: bus type drm_connector registered ... this line here above. It is the last one output. What you see here below what I'm catching from serial. [8.456734] Console: switching to colour dummy device 80x25 [8.464414] nouveau :03:00.0: vgaarb: deactivate vga console [8.473063] nouveau :03:00.0: NVIDIA GT218 (0a8c00b1) [8.594096] nouveau :03:00.0: bios: version 70.18.83.00.08 [8.607906] nouveau :03:00.0: fb: 512 MiB DDR3 [8.926721] nouveau :03:00.0: DRM: VRAM: 512 MiB [8.931763] nouveau :03:00.0: DRM: GART: 1048576 MiB [8.937156] nouveau :03:00.0: DRM: TMDS table version 2.0 [8.942969] nouveau :03:00.0: DRM: DCB version 4.0 [8.948173] nouveau :03:00.0: DRM: DCB outp 00: 02000360 [8.954696] nouveau :03:00.0: DRM: DCB outp 01: 02000362 00020010 [8.961211] nouveau :03:00.0: DRM: DCB outp 02: 028003a6 0f220010 [8.967739] nouveau :03:00.0: DRM: DCB outp 03: 01011380 [8.974261] nouveau :03:00.0: DRM: DCB outp 04: 08011382 00020010 [8.980769] nouveau :03:00.0: DRM: DCB outp 05: 088113c6 0f220010 [8.987293] nouveau :03:00.0: DRM: DCB conn 00: 00101064 [8.993015] nouveau :03:00.0: DRM: DCB conn 01: 00202165 [9.005724] nouveau :03:00.0: DRM: MM: using COPY for buffer copies [9.023889] [drm] Initialized nouveau 1.3.1 20120801 for :03:00.0 on minor 0 [9.032044] nouveau :03:00.0: [drm] Cannot find any crtc or sizes [9.162909] megasas: 07.725.01.00-rc1 [9.167537] st: Version 20160209, fixed bufsize 32768, s/g segs 256 [9.176058] ahci :00:1f.2: version 3.0 [9.194078] ahci :00:1f.2: AHCI 0001.0300 32 slots 6 ports 6 Gbps 0x3 impl SATA mode [9.202487] ahci :00:1f.2: flags: 64bit ncq sntf pm led clo pio slum part ems apst [9.243154] scsi host0: ahci [9.252090] scsi host1: ahci [9.260389] scsi host2: ahci [9.268061] scsi host3: ahci [9.273542] scsi host4: ahci [9.279071] scsi host5: ahci ... and so on until full boot. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 1/9] drm/sched: Convert drm scheduler to use a work queue rather than kthread
On Wed, Aug 16, 2023 at 02:30:38PM +0200, Danilo Krummrich wrote: > On 8/16/23 16:05, Christian König wrote: > > Am 16.08.23 um 13:30 schrieb Danilo Krummrich: > > > Hi Matt, > > > > > > On 8/11/23 04:31, Matthew Brost wrote: > > > > In XE, the new Intel GPU driver, a choice has made to have a 1 to 1 > > > > mapping between a drm_gpu_scheduler and drm_sched_entity. At first this > > > > seems a bit odd but let us explain the reasoning below. > > > > > > > > 1. In XE the submission order from multiple drm_sched_entity is not > > > > guaranteed to be the same completion even if targeting the same hardware > > > > engine. This is because in XE we have a firmware scheduler, the GuC, > > > > which allowed to reorder, timeslice, and preempt submissions. If a using > > > > shared drm_gpu_scheduler across multiple drm_sched_entity, the TDR falls > > > > apart as the TDR expects submission order == completion order. Using a > > > > dedicated drm_gpu_scheduler per drm_sched_entity solve this problem. > > > > > > > > 2. In XE submissions are done via programming a ring buffer (circular > > > > buffer), a drm_gpu_scheduler provides a limit on number of jobs, if the > > > > limit of number jobs is set to RING_SIZE / MAX_SIZE_PER_JOB we get flow > > > > control on the ring for free. > > > > > > In XE, where does the limitation of MAX_SIZE_PER_JOB come from? > > > In Xe the job submission is series of ring instructions done by the KMD. The instructions are cache flushes, seqno writes, jump to user BB, etc... The exact instructions for each job vary based on hw engine type, platform, etc... We dervive MAX_SIZE_PER_JOB from the largest set of instructions to submit a job and have a define in the driver for this. I believe it is currently set to 192 bytes (the actual define is MAX_JOB_SIZE_BYTES). So a 16k ring lets Xe have 85 jobs inflight at once. > > > In Nouveau we currently do have such a limitation as well, but it is > > > derived from the RING_SIZE, hence RING_SIZE / MAX_SIZE_PER_JOB would > > > always be 1. However, I think most jobs won't actually utilize the > > > whole ring. > > > > Well that should probably rather be RING_SIZE / MAX_SIZE_PER_JOB = > > hw_submission_limit (or even hw_submission_limit - 1 when the hw can't Yes, hw_submission_limit = RING_SIZE / MAX_SIZE_PER_JOB in Xe. > > distinct full vs empty ring buffer). > > Not sure if I get you right, let me try to clarify what I was trying to say: > I wanted to say that in Nouveau MAX_SIZE_PER_JOB isn't really limited by > anything other than the RING_SIZE and hence we'd never allow more than 1 > active job. I'm confused how there isn't a limit on the size of the job in Nouveau? Based on what you have said, a job could be larger than the ring then? > > However, it seems to be more efficient to base ring flow control on the > actual size of each incoming job rather than the worst case, namely the > maximum size of a job. > If this doesn't work for Nouveau, feel free flow control the ring differently but this works rather well (and simple) for Xe. Matt > > > > Otherwise your scheduler might just overwrite the ring buffer by pushing > > things to fast. > > > > Christian. > > > > > > > > Given that, it seems like it would be better to let the scheduler > > > keep track of empty ring "slots" instead, such that the scheduler > > > can deceide whether a subsequent job will still fit on the ring and > > > if not re-evaluate once a previous job finished. Of course each > > > submitted job would be required to carry the number of slots it > > > requires on the ring. > > > > > > What to you think of implementing this as alternative flow control > > > mechanism? Implementation wise this could be a union with the > > > existing hw_submission_limit. > > > > > > - Danilo > > > > > > > > > > > A problem with this design is currently a drm_gpu_scheduler uses a > > > > kthread for submission / job cleanup. This doesn't scale if a large > > > > number of drm_gpu_scheduler are used. To work around the scaling issue, > > > > use a worker rather than kthread for submission / job cleanup. > > > > > > > > v2: > > > > - (Rob Clark) Fix msm build > > > > - Pass in run work queue > > > > v3: > > > > - (Boris) don't have loop in worker > > > > v4: > > > > - (Tvrtko) break out submit ready, stop, start helpers into own patch > > > > > > > > Signed-off-by: Matthew Brost > > > > > >
Re: [PATCH v2 1/9] drm/sched: Convert drm scheduler to use a work queue rather than kthread
On 8/16/23 16:05, Christian König wrote: Am 16.08.23 um 13:30 schrieb Danilo Krummrich: Hi Matt, On 8/11/23 04:31, Matthew Brost wrote: In XE, the new Intel GPU driver, a choice has made to have a 1 to 1 mapping between a drm_gpu_scheduler and drm_sched_entity. At first this seems a bit odd but let us explain the reasoning below. 1. In XE the submission order from multiple drm_sched_entity is not guaranteed to be the same completion even if targeting the same hardware engine. This is because in XE we have a firmware scheduler, the GuC, which allowed to reorder, timeslice, and preempt submissions. If a using shared drm_gpu_scheduler across multiple drm_sched_entity, the TDR falls apart as the TDR expects submission order == completion order. Using a dedicated drm_gpu_scheduler per drm_sched_entity solve this problem. 2. In XE submissions are done via programming a ring buffer (circular buffer), a drm_gpu_scheduler provides a limit on number of jobs, if the limit of number jobs is set to RING_SIZE / MAX_SIZE_PER_JOB we get flow control on the ring for free. In XE, where does the limitation of MAX_SIZE_PER_JOB come from? In Nouveau we currently do have such a limitation as well, but it is derived from the RING_SIZE, hence RING_SIZE / MAX_SIZE_PER_JOB would always be 1. However, I think most jobs won't actually utilize the whole ring. Well that should probably rather be RING_SIZE / MAX_SIZE_PER_JOB = hw_submission_limit (or even hw_submission_limit - 1 when the hw can't distinct full vs empty ring buffer). Not sure if I get you right, let me try to clarify what I was trying to say: I wanted to say that in Nouveau MAX_SIZE_PER_JOB isn't really limited by anything other than the RING_SIZE and hence we'd never allow more than 1 active job. However, it seems to be more efficient to base ring flow control on the actual size of each incoming job rather than the worst case, namely the maximum size of a job. Otherwise your scheduler might just overwrite the ring buffer by pushing things to fast. Christian. Given that, it seems like it would be better to let the scheduler keep track of empty ring "slots" instead, such that the scheduler can deceide whether a subsequent job will still fit on the ring and if not re-evaluate once a previous job finished. Of course each submitted job would be required to carry the number of slots it requires on the ring. What to you think of implementing this as alternative flow control mechanism? Implementation wise this could be a union with the existing hw_submission_limit. - Danilo A problem with this design is currently a drm_gpu_scheduler uses a kthread for submission / job cleanup. This doesn't scale if a large number of drm_gpu_scheduler are used. To work around the scaling issue, use a worker rather than kthread for submission / job cleanup. v2: - (Rob Clark) Fix msm build - Pass in run work queue v3: - (Boris) don't have loop in worker v4: - (Tvrtko) break out submit ready, stop, start helpers into own patch Signed-off-by: Matthew Brost
[Bug 217797] [amdgpu/mm?] HSA_AMD_SVM=y causes/triggers PAT issues
https://bugzilla.kernel.org/show_bug.cgi?id=217797 Artem S. Tashkinov (a...@gmx.com) changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |ANSWERED --- Comment #1 from Artem S. Tashkinov (a...@gmx.com) --- Please report here instead: https://gitlab.freedesktop.org/drm/amd/-/issues -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH v2] drm/amdgpu: register a dirty framebuffer callback for fbcon
Am 16.08.23 um 15:41 schrieb Hamza Mahfooz: On 8/16/23 01:55, Christian König wrote: Am 15.08.23 um 19:26 schrieb Hamza Mahfooz: fbcon requires that we implement _framebuffer_funcs.dirty. Otherwise, the framebuffer might take a while to flush (which would manifest as noticeable lag). However, we can't enable this callback for non-fbcon cases since it might cause too many atomic commits to be made at once. So, implement amdgpu_dirtyfb() and only enable it for fbcon framebuffers on devices that support atomic KMS. Cc: Aurabindo Pillai Cc: Mario Limonciello Cc: sta...@vger.kernel.org # 6.1+ Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2519 Signed-off-by: Hamza Mahfooz --- v2: update variable names --- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 26 - 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c index d20dd3f852fc..d3b59f99cb7c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c @@ -38,6 +38,8 @@ #include #include #include +#include +#include #include #include #include @@ -532,11 +534,29 @@ bool amdgpu_display_ddc_probe(struct amdgpu_connector *amdgpu_connector, return true; } +static int amdgpu_dirtyfb(struct drm_framebuffer *fb, struct drm_file *file, + unsigned int flags, unsigned int color, + struct drm_clip_rect *clips, unsigned int num_clips) +{ + + if (strcmp(fb->comm, "[fbcon]")) + return -ENOSYS; Once more to the v2 of this patch: Tests like those are a pretty big NO-GO for upstreaming. On closer inspection it is actually sufficient to check if `file` is NULL here (since it means that the request isn't from userspace). So, do you think that would be palatable for upstream? That's certainly better than doing a string compare, but I'm not sure if that's sufficient. In general drivers shouldn't have any special handling for fdcon. You should probably have Thomas Zimmermann take a look at this. Regards, Christian. Regards, Christian. + + return drm_atomic_helper_dirtyfb(fb, file, flags, color, clips, + num_clips); +} + static const struct drm_framebuffer_funcs amdgpu_fb_funcs = { .destroy = drm_gem_fb_destroy, .create_handle = drm_gem_fb_create_handle, }; +static const struct drm_framebuffer_funcs amdgpu_fb_funcs_atomic = { + .destroy = drm_gem_fb_destroy, + .create_handle = drm_gem_fb_create_handle, + .dirty = amdgpu_dirtyfb +}; + uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev, uint64_t bo_flags) { @@ -1139,7 +1159,11 @@ static int amdgpu_display_gem_fb_verify_and_init(struct drm_device *dev, if (ret) goto err; - ret = drm_framebuffer_init(dev, >base, _fb_funcs); + if (drm_drv_uses_atomic_modeset(dev)) + ret = drm_framebuffer_init(dev, >base, + _fb_funcs_atomic); + else + ret = drm_framebuffer_init(dev, >base, _fb_funcs); if (ret) goto err;
Re: [PATCH v2 1/9] drm/sched: Convert drm scheduler to use a work queue rather than kthread
Am 16.08.23 um 13:30 schrieb Danilo Krummrich: Hi Matt, On 8/11/23 04:31, Matthew Brost wrote: In XE, the new Intel GPU driver, a choice has made to have a 1 to 1 mapping between a drm_gpu_scheduler and drm_sched_entity. At first this seems a bit odd but let us explain the reasoning below. 1. In XE the submission order from multiple drm_sched_entity is not guaranteed to be the same completion even if targeting the same hardware engine. This is because in XE we have a firmware scheduler, the GuC, which allowed to reorder, timeslice, and preempt submissions. If a using shared drm_gpu_scheduler across multiple drm_sched_entity, the TDR falls apart as the TDR expects submission order == completion order. Using a dedicated drm_gpu_scheduler per drm_sched_entity solve this problem. 2. In XE submissions are done via programming a ring buffer (circular buffer), a drm_gpu_scheduler provides a limit on number of jobs, if the limit of number jobs is set to RING_SIZE / MAX_SIZE_PER_JOB we get flow control on the ring for free. In XE, where does the limitation of MAX_SIZE_PER_JOB come from? In Nouveau we currently do have such a limitation as well, but it is derived from the RING_SIZE, hence RING_SIZE / MAX_SIZE_PER_JOB would always be 1. However, I think most jobs won't actually utilize the whole ring. Well that should probably rather be RING_SIZE / MAX_SIZE_PER_JOB = hw_submission_limit (or even hw_submission_limit - 1 when the hw can't distinct full vs empty ring buffer). Otherwise your scheduler might just overwrite the ring buffer by pushing things to fast. Christian. Given that, it seems like it would be better to let the scheduler keep track of empty ring "slots" instead, such that the scheduler can deceide whether a subsequent job will still fit on the ring and if not re-evaluate once a previous job finished. Of course each submitted job would be required to carry the number of slots it requires on the ring. What to you think of implementing this as alternative flow control mechanism? Implementation wise this could be a union with the existing hw_submission_limit. - Danilo A problem with this design is currently a drm_gpu_scheduler uses a kthread for submission / job cleanup. This doesn't scale if a large number of drm_gpu_scheduler are used. To work around the scaling issue, use a worker rather than kthread for submission / job cleanup. v2: - (Rob Clark) Fix msm build - Pass in run work queue v3: - (Boris) don't have loop in worker v4: - (Tvrtko) break out submit ready, stop, start helpers into own patch Signed-off-by: Matthew Brost
Re: [PATCH 1/5] mm: move some shrinker-related function declarations to mm/internal.h
Hi Qi, kernel test robot noticed the following build warnings: [auto build test WARNING on brauner-vfs/vfs.all] [also build test WARNING on linus/master v6.5-rc6 next-20230816] [cannot apply to akpm-mm/mm-everything drm-misc/drm-misc-next vfs-idmapping/for-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Qi-Zheng/mm-move-some-shrinker-related-function-declarations-to-mm-internal-h/20230816-163833 base: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.all patch link: https://lore.kernel.org/r/20230816083419.41088-2-zhengqi.arch%40bytedance.com patch subject: [PATCH 1/5] mm: move some shrinker-related function declarations to mm/internal.h config: m68k-randconfig-r013-20230816 (https://download.01.org/0day-ci/archive/20230816/202308162105.y9xrlta7-...@intel.com/config) compiler: m68k-linux-gcc (GCC) 12.3.0 reproduce: (https://download.01.org/0day-ci/archive/20230816/202308162105.y9xrlta7-...@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/202308162105.y9xrlta7-...@intel.com/ All warnings (new ones prefixed by >>): >> mm/shrinker_debug.c:174:5: warning: no previous prototype for >> 'shrinker_debugfs_add' [-Wmissing-prototypes] 174 | int shrinker_debugfs_add(struct shrinker *shrinker) | ^~~~ >> mm/shrinker_debug.c:249:16: warning: no previous prototype for >> 'shrinker_debugfs_detach' [-Wmissing-prototypes] 249 | struct dentry *shrinker_debugfs_detach(struct shrinker *shrinker, |^~~ >> mm/shrinker_debug.c:265:6: warning: no previous prototype for >> 'shrinker_debugfs_remove' [-Wmissing-prototypes] 265 | void shrinker_debugfs_remove(struct dentry *debugfs_entry, int debugfs_id) | ^~~ vim +/shrinker_debugfs_add +174 mm/shrinker_debug.c bbf535fd6f06b9 Roman Gushchin 2022-05-31 173 5035ebc644aec9 Roman Gushchin 2022-05-31 @174 int shrinker_debugfs_add(struct shrinker *shrinker) 5035ebc644aec9 Roman Gushchin 2022-05-31 175 { 5035ebc644aec9 Roman Gushchin 2022-05-31 176 struct dentry *entry; e33c267ab70de4 Roman Gushchin 2022-05-31 177 char buf[128]; 5035ebc644aec9 Roman Gushchin 2022-05-31 178 int id; 5035ebc644aec9 Roman Gushchin 2022-05-31 179 47a7c01c3efc65 Qi Zheng 2023-06-09 180 lockdep_assert_held(_rwsem); 5035ebc644aec9 Roman Gushchin 2022-05-31 181 5035ebc644aec9 Roman Gushchin 2022-05-31 182 /* debugfs isn't initialized yet, add debugfs entries later. */ 5035ebc644aec9 Roman Gushchin 2022-05-31 183 if (!shrinker_debugfs_root) 5035ebc644aec9 Roman Gushchin 2022-05-31 184 return 0; 5035ebc644aec9 Roman Gushchin 2022-05-31 185 5035ebc644aec9 Roman Gushchin 2022-05-31 186 id = ida_alloc(_debugfs_ida, GFP_KERNEL); 5035ebc644aec9 Roman Gushchin 2022-05-31 187 if (id < 0) 5035ebc644aec9 Roman Gushchin 2022-05-31 188 return id; 5035ebc644aec9 Roman Gushchin 2022-05-31 189 shrinker->debugfs_id = id; 5035ebc644aec9 Roman Gushchin 2022-05-31 190 e33c267ab70de4 Roman Gushchin 2022-05-31 191 snprintf(buf, sizeof(buf), "%s-%d", shrinker->name, id); 5035ebc644aec9 Roman Gushchin 2022-05-31 192 5035ebc644aec9 Roman Gushchin 2022-05-31 193 /* create debugfs entry */ 5035ebc644aec9 Roman Gushchin 2022-05-31 194 entry = debugfs_create_dir(buf, shrinker_debugfs_root); 5035ebc644aec9 Roman Gushchin 2022-05-31 195 if (IS_ERR(entry)) { 5035ebc644aec9 Roman Gushchin 2022-05-31 196 ida_free(_debugfs_ida, id); 5035ebc644aec9 Roman Gushchin 2022-05-31 197 return PTR_ERR(entry); 5035ebc644aec9 Roman Gushchin 2022-05-31 198 } 5035ebc644aec9 Roman Gushchin 2022-05-31 199 shrinker->debugfs_entry = entry; 5035ebc644aec9 Roman Gushchin 2022-05-31 200 2124f79de6a909 John Keeping 2023-04-18 201 debugfs_create_file("count", 0440, entry, shrinker, 5035ebc644aec9 Roman Gushchin 2022-05-31 202 _debugfs_count_fops); 2124f79de6a909 John Keeping 2023-04-18 203 debugfs_create_file("scan", 0220, entry, shrinker, bbf535fd6f06b9 Roman Gushchin 2022-05-31 204 _debugfs_scan_fops); 5035ebc644aec9 Roman Gushchin 2022-05-31 205 return 0; 5035ebc644aec9 Roman Gushchin 2022-05-31 206 } 5035ebc64
Re: [PATCH v2] drm/amdgpu: register a dirty framebuffer callback for fbcon
On 8/16/23 01:55, Christian König wrote: Am 15.08.23 um 19:26 schrieb Hamza Mahfooz: fbcon requires that we implement _framebuffer_funcs.dirty. Otherwise, the framebuffer might take a while to flush (which would manifest as noticeable lag). However, we can't enable this callback for non-fbcon cases since it might cause too many atomic commits to be made at once. So, implement amdgpu_dirtyfb() and only enable it for fbcon framebuffers on devices that support atomic KMS. Cc: Aurabindo Pillai Cc: Mario Limonciello Cc: sta...@vger.kernel.org # 6.1+ Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2519 Signed-off-by: Hamza Mahfooz --- v2: update variable names --- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 26 - 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c index d20dd3f852fc..d3b59f99cb7c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c @@ -38,6 +38,8 @@ #include #include #include +#include +#include #include #include #include @@ -532,11 +534,29 @@ bool amdgpu_display_ddc_probe(struct amdgpu_connector *amdgpu_connector, return true; } +static int amdgpu_dirtyfb(struct drm_framebuffer *fb, struct drm_file *file, + unsigned int flags, unsigned int color, + struct drm_clip_rect *clips, unsigned int num_clips) +{ + + if (strcmp(fb->comm, "[fbcon]")) + return -ENOSYS; Once more to the v2 of this patch: Tests like those are a pretty big NO-GO for upstreaming. On closer inspection it is actually sufficient to check if `file` is NULL here (since it means that the request isn't from userspace). So, do you think that would be palatable for upstream? Regards, Christian. + + return drm_atomic_helper_dirtyfb(fb, file, flags, color, clips, + num_clips); +} + static const struct drm_framebuffer_funcs amdgpu_fb_funcs = { .destroy = drm_gem_fb_destroy, .create_handle = drm_gem_fb_create_handle, }; +static const struct drm_framebuffer_funcs amdgpu_fb_funcs_atomic = { + .destroy = drm_gem_fb_destroy, + .create_handle = drm_gem_fb_create_handle, + .dirty = amdgpu_dirtyfb +}; + uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev, uint64_t bo_flags) { @@ -1139,7 +1159,11 @@ static int amdgpu_display_gem_fb_verify_and_init(struct drm_device *dev, if (ret) goto err; - ret = drm_framebuffer_init(dev, >base, _fb_funcs); + if (drm_drv_uses_atomic_modeset(dev)) + ret = drm_framebuffer_init(dev, >base, + _fb_funcs_atomic); + else + ret = drm_framebuffer_init(dev, >base, _fb_funcs); if (ret) goto err; -- Hamza
Re: [PATCH] Documentation/gpu: Update amdgpu documentation
[AMD Official Use Only - General] Reviewed-by: Alex Deucher From: Lazar, Lijo Sent: Tuesday, August 15, 2023 11:56 PM To: amd-...@lists.freedesktop.org Cc: Zhang, Hawking ; Deucher, Alexander ; s...@canb.auug.org.au ; airl...@redhat.com ; dri-devel@lists.freedesktop.org Subject: [PATCH] Documentation/gpu: Update amdgpu documentation 7957ec80ef97 ("drm/amdgpu: Add FRU sysfs nodes only if needed") moved the documentation for some of the sysfs nodes to amdgpu_fru_eeprom.c. Update the documentation accordingly. Signed-off-by: Lijo Lazar --- Documentation/gpu/amdgpu/driver-misc.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/gpu/amdgpu/driver-misc.rst b/Documentation/gpu/amdgpu/driver-misc.rst index be131e963d87..26334e54447b 100644 --- a/Documentation/gpu/amdgpu/driver-misc.rst +++ b/Documentation/gpu/amdgpu/driver-misc.rst @@ -11,19 +11,19 @@ via sysfs product_name -.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c :doc: product_name product_number -- -.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c :doc: product_name serial_number - -.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c :doc: serial_number unique_id -- 2.25.1
Re: [PATCH v2 1/9] drm/sched: Convert drm scheduler to use a work queue rather than kthread
Hi Matt, On 8/11/23 04:31, Matthew Brost wrote: In XE, the new Intel GPU driver, a choice has made to have a 1 to 1 mapping between a drm_gpu_scheduler and drm_sched_entity. At first this seems a bit odd but let us explain the reasoning below. 1. In XE the submission order from multiple drm_sched_entity is not guaranteed to be the same completion even if targeting the same hardware engine. This is because in XE we have a firmware scheduler, the GuC, which allowed to reorder, timeslice, and preempt submissions. If a using shared drm_gpu_scheduler across multiple drm_sched_entity, the TDR falls apart as the TDR expects submission order == completion order. Using a dedicated drm_gpu_scheduler per drm_sched_entity solve this problem. 2. In XE submissions are done via programming a ring buffer (circular buffer), a drm_gpu_scheduler provides a limit on number of jobs, if the limit of number jobs is set to RING_SIZE / MAX_SIZE_PER_JOB we get flow control on the ring for free. In XE, where does the limitation of MAX_SIZE_PER_JOB come from? In Nouveau we currently do have such a limitation as well, but it is derived from the RING_SIZE, hence RING_SIZE / MAX_SIZE_PER_JOB would always be 1. However, I think most jobs won't actually utilize the whole ring. Given that, it seems like it would be better to let the scheduler keep track of empty ring "slots" instead, such that the scheduler can deceide whether a subsequent job will still fit on the ring and if not re-evaluate once a previous job finished. Of course each submitted job would be required to carry the number of slots it requires on the ring. What to you think of implementing this as alternative flow control mechanism? Implementation wise this could be a union with the existing hw_submission_limit. - Danilo A problem with this design is currently a drm_gpu_scheduler uses a kthread for submission / job cleanup. This doesn't scale if a large number of drm_gpu_scheduler are used. To work around the scaling issue, use a worker rather than kthread for submission / job cleanup. v2: - (Rob Clark) Fix msm build - Pass in run work queue v3: - (Boris) don't have loop in worker v4: - (Tvrtko) break out submit ready, stop, start helpers into own patch Signed-off-by: Matthew Brost
Re: [PATCH 1/5] mm: move some shrinker-related function declarations to mm/internal.h
Hi Qi, kernel test robot noticed the following build warnings: [auto build test WARNING on brauner-vfs/vfs.all] [also build test WARNING on linus/master v6.5-rc6 next-20230816] [cannot apply to akpm-mm/mm-everything drm-misc/drm-misc-next vfs-idmapping/for-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Qi-Zheng/mm-move-some-shrinker-related-function-declarations-to-mm-internal-h/20230816-163833 base: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.all patch link: https://lore.kernel.org/r/20230816083419.41088-2-zhengqi.arch%40bytedance.com patch subject: [PATCH 1/5] mm: move some shrinker-related function declarations to mm/internal.h config: riscv-randconfig-r015-20230816 (https://download.01.org/0day-ci/archive/20230816/202308162118.motjd6ag-...@intel.com/config) compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07) reproduce: (https://download.01.org/0day-ci/archive/20230816/202308162118.motjd6ag-...@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/202308162118.motjd6ag-...@intel.com/ All warnings (new ones prefixed by >>): ~~ ^ In file included from mm/shrinker_debug.c:7: In file included from include/linux/memcontrol.h:13: In file included from include/linux/cgroup.h:26: In file included from include/linux/kernel_stat.h:9: In file included from include/linux/interrupt.h:11: In file included from include/linux/hardirq.h:11: In file included from ./arch/riscv/include/generated/asm/hardirq.h:1: In file included from include/asm-generic/hardirq.h:17: In file included from include/linux/irq.h:20: In file included from include/linux/io.h:13: In file included from arch/riscv/include/asm/io.h:136: include/asm-generic/io.h:751:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] insw(addr, buffer, count); ^ arch/riscv/include/asm/io.h:105:53: note: expanded from macro 'insw' #define insw(addr, buffer, count) __insw(PCI_IOBASE + (addr), buffer, count) ~~ ^ In file included from mm/shrinker_debug.c:7: In file included from include/linux/memcontrol.h:13: In file included from include/linux/cgroup.h:26: In file included from include/linux/kernel_stat.h:9: In file included from include/linux/interrupt.h:11: In file included from include/linux/hardirq.h:11: In file included from ./arch/riscv/include/generated/asm/hardirq.h:1: In file included from include/asm-generic/hardirq.h:17: In file included from include/linux/irq.h:20: In file included from include/linux/io.h:13: In file included from arch/riscv/include/asm/io.h:136: include/asm-generic/io.h:759:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] insl(addr, buffer, count); ^ arch/riscv/include/asm/io.h:106:53: note: expanded from macro 'insl' #define insl(addr, buffer, count) __insl(PCI_IOBASE + (addr), buffer, count) ~~ ^ In file included from mm/shrinker_debug.c:7: In file included from include/linux/memcontrol.h:13: In file included from include/linux/cgroup.h:26: In file included from include/linux/kernel_stat.h:9: In file included from include/linux/interrupt.h:11: In file included from include/linux/hardirq.h:11: In file included from ./arch/riscv/include/generated/asm/hardirq.h:1: In file included from include/asm-generic/hardirq.h:17: In file included from include/linux/irq.h:20: In file included from include/linux/io.h:13: In file included from arch/riscv/include/asm/io.h:136: include/asm-generic/io.h:768:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] outsb(addr, buffer, count); ^~ arch/riscv/include/asm/io.h:118:55: note: expanded from macro 'outsb' #define outsb(addr, buffer, count) __outsb(PCI_IOBASE + (addr), buffer, count) ~~ ^ In file included from mm/shrinker_debug.c:7: In file included from include/linux/memcontrol.h:13: In file included from include/linux/cgroup.h:26: In file included from include/linux/kernel_stat.h:9: In file included from include/linux/interrupt.h:11: I
[Bug 217664] Laptop doesnt wake up from suspend mode.
https://bugzilla.kernel.org/show_bug.cgi?id=217664 Alex Deucher (alexdeuc...@gmail.com) changed: What|Removed |Added CC||alexdeuc...@gmail.com --- Comment #9 from Alex Deucher (alexdeuc...@gmail.com) --- Does this laptop use a display mux? Does it work without the nvidia driver loaded? Can you suspend via something other than the lid (e.g., via the menu in your GUI)? Does that work? -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[PATCH v2 10/12] drm/bridge: tc358768: Fix tc358768_ns_to_cnt()
The tc358768_ns_to_cnt() is, most likely, supposed to do a div-round-up operation, but it misses subtracting one from the dividend. Fix this by just using DIV_ROUND_UP(). Fixes: ff1ca6397b1d ("drm/bridge: Add tc358768 driver") Signed-off-by: Tomi Valkeinen --- drivers/gpu/drm/bridge/tc358768.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c index 9411b0fb471e..dc2241c18dde 100644 --- a/drivers/gpu/drm/bridge/tc358768.c +++ b/drivers/gpu/drm/bridge/tc358768.c @@ -630,7 +630,7 @@ static int tc358768_setup_pll(struct tc358768_priv *priv, static u32 tc358768_ns_to_cnt(u32 ns, u32 period_ps) { - return (ns * 1000 + period_ps) / period_ps; + return DIV_ROUND_UP(ns * 1000, period_ps); } static u32 tc358768_ps_to_ns(u32 ps) -- 2.34.1
[PATCH v2 09/12] drm/bridge: tc358768: Clean up clock period code
The driver defines TC358768_PRECISION as 1000, and uses "nsk" to refer to clock periods. The original author does not remember where all this came from. Effectively the driver is using picoseconds as the unit for clock periods, yet referring to them by "nsk". Clean this up by just saying the periods are in picoseconds. Signed-off-by: Tomi Valkeinen --- drivers/gpu/drm/bridge/tc358768.c | 60 +++ 1 file changed, 29 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c index db45b4a982c0..9411b0fb471e 100644 --- a/drivers/gpu/drm/bridge/tc358768.c +++ b/drivers/gpu/drm/bridge/tc358768.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -627,15 +628,14 @@ static int tc358768_setup_pll(struct tc358768_priv *priv, return tc358768_clear_error(priv); } -#define TC358768_PRECISION 1000 -static u32 tc358768_ns_to_cnt(u32 ns, u32 period_nsk) +static u32 tc358768_ns_to_cnt(u32 ns, u32 period_ps) { - return (ns * TC358768_PRECISION + period_nsk) / period_nsk; + return (ns * 1000 + period_ps) / period_ps; } -static u32 tc358768_to_ns(u32 nsk) +static u32 tc358768_ps_to_ns(u32 ps) { - return (nsk / TC358768_PRECISION); + return ps / 1000; } static void tc358768_bridge_pre_enable(struct drm_bridge *bridge) @@ -646,7 +646,7 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge) u32 val, val2, lptxcnt, hact, data_type; s32 raw_val; const struct drm_display_mode *mode; - u32 hsbyteclk_nsk, dsiclk_nsk, ui_nsk; + u32 hsbyteclk_ps, dsiclk_ps, ui_ps; u32 dsiclk, hsbyteclk, video_start; const u32 internal_delay = 40; int ret, i; @@ -730,67 +730,65 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge) tc358768_write(priv, TC358768_D0W_CNTRL + i * 4, 0x); /* DSI Timings */ - hsbyteclk_nsk = (u32)div_u64((u64)10 * TC358768_PRECISION, - hsbyteclk); - dsiclk_nsk = (u32)div_u64((u64)10 * TC358768_PRECISION, dsiclk); - ui_nsk = dsiclk_nsk / 2; - dev_dbg(dev, "dsiclk_nsk: %u\n", dsiclk_nsk); - dev_dbg(dev, "ui_nsk: %u\n", ui_nsk); - dev_dbg(dev, "hsbyteclk_nsk: %u\n", hsbyteclk_nsk); + hsbyteclk_ps = (u32)div_u64(PICO, hsbyteclk); + dsiclk_ps = (u32)div_u64(PICO, dsiclk); + ui_ps = dsiclk_ps / 2; + dev_dbg(dev, "dsiclk: %u ps, ui %u ps, hsbyteclk %u ps\n", dsiclk_ps, + ui_ps, hsbyteclk_ps); /* LP11 > 100us for D-PHY Rx Init */ - val = tc358768_ns_to_cnt(100 * 1000, hsbyteclk_nsk) - 1; + val = tc358768_ns_to_cnt(100 * 1000, hsbyteclk_ps) - 1; dev_dbg(dev, "LINEINITCNT: %u\n", val); tc358768_write(priv, TC358768_LINEINITCNT, val); /* LPTimeCnt > 50ns */ - val = tc358768_ns_to_cnt(50, hsbyteclk_nsk) - 1; + val = tc358768_ns_to_cnt(50, hsbyteclk_ps) - 1; lptxcnt = val; dev_dbg(dev, "LPTXTIMECNT: %u\n", val); tc358768_write(priv, TC358768_LPTXTIMECNT, val); /* 38ns < TCLK_PREPARE < 95ns */ - val = tc358768_ns_to_cnt(65, hsbyteclk_nsk) - 1; + val = tc358768_ns_to_cnt(65, hsbyteclk_ps) - 1; dev_dbg(dev, "TCLK_PREPARECNT %u\n", val); /* TCLK_PREPARE + TCLK_ZERO > 300ns */ - val2 = tc358768_ns_to_cnt(300 - tc358768_to_ns(2 * ui_nsk), - hsbyteclk_nsk) - 2; + val2 = tc358768_ns_to_cnt(300 - tc358768_ps_to_ns(2 * ui_ps), + hsbyteclk_ps) - 2; dev_dbg(dev, "TCLK_ZEROCNT %u\n", val2); val |= val2 << 8; tc358768_write(priv, TC358768_TCLK_HEADERCNT, val); /* TCLK_TRAIL > 60ns AND TEOT <= 105 ns + 12*UI */ - raw_val = tc358768_ns_to_cnt(60 + tc358768_to_ns(2 * ui_nsk), hsbyteclk_nsk) - 5; + raw_val = tc358768_ns_to_cnt(60 + tc358768_ps_to_ns(2 * ui_ps), hsbyteclk_ps) - 5; val = clamp(raw_val, 0, 127); dev_dbg(dev, "TCLK_TRAILCNT: %u\n", val); tc358768_write(priv, TC358768_TCLK_TRAILCNT, val); /* 40ns + 4*UI < THS_PREPARE < 85ns + 6*UI */ - val = 50 + tc358768_to_ns(4 * ui_nsk); - val = tc358768_ns_to_cnt(val, hsbyteclk_nsk) - 1; + val = 50 + tc358768_ps_to_ns(4 * ui_ps); + val = tc358768_ns_to_cnt(val, hsbyteclk_ps) - 1; dev_dbg(dev, "THS_PREPARECNT %u\n", val); /* THS_PREPARE + THS_ZERO > 145ns + 10*UI */ - raw_val = tc358768_ns_to_cnt(145 - tc358768_to_ns(3 * ui_nsk), hsbyteclk_nsk) - 10; + raw_val = tc358768_ns_to_cnt(145 - tc358768_ps_to_ns(3 * ui_ps), hsbyteclk_ps) - 10; val2 = clamp(raw_val, 0, 127); dev_dbg(dev, "THS_ZEROCNT %u\n", val2); val |= val2 << 8; tc358768_write(priv, TC358768_THS_HEADERCNT, val); /* TWAKEUP > 1ms in lptxcnt steps */ - val
[PATCH v2 12/12] drm/bridge: tc358768: Default to positive h/v syncs
As the TC358768 is a DPI to DSI bridge, the DSI side does not need to define h/v sync polarities. This means that sometimes we have a mode without defined sync polarities, which does not work on the DPI side. Add a mode_fixup hook to default to positive sync polarities. Signed-off-by: Tomi Valkeinen --- drivers/gpu/drm/bridge/tc358768.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c index ea19de5509ed..b465e0a31d09 100644 --- a/drivers/gpu/drm/bridge/tc358768.c +++ b/drivers/gpu/drm/bridge/tc358768.c @@ -1124,9 +1124,27 @@ tc358768_atomic_get_input_bus_fmts(struct drm_bridge *bridge, return input_fmts; } +static bool tc358768_mode_fixup(struct drm_bridge *bridge, + const struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode) +{ + /* Default to positive sync */ + + if (!(adjusted_mode->flags & + (DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NHSYNC))) + adjusted_mode->flags |= DRM_MODE_FLAG_PHSYNC; + + if (!(adjusted_mode->flags & + (DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_NVSYNC))) + adjusted_mode->flags |= DRM_MODE_FLAG_PVSYNC; + + return true; +} + static const struct drm_bridge_funcs tc358768_bridge_funcs = { .attach = tc358768_bridge_attach, .mode_valid = tc358768_bridge_mode_valid, + .mode_fixup = tc358768_mode_fixup, .pre_enable = tc358768_bridge_pre_enable, .enable = tc358768_bridge_enable, .disable = tc358768_bridge_disable, -- 2.34.1
[PATCH v2 11/12] drm/bridge: tc358768: Attempt to fix DSI horizontal timings
The DSI horizontal timing calculations done by the driver seem to often lead to underflows or overflows, depending on the videomode. There are two main things the current driver doesn't seem to get right: DSI HSW and HFP, and VSDly. However, even following Toshiba's documentation it seems we don't always get a working display. This patch attempts to fix the horizontal timings for DSI event mode, and on a system with a DSI->HDMI encoder, a lot of standard HDMI modes now seem to work. The work relies on Toshiba's documentation, but also quite a bit on empirical testing. This also adds timing related debug prints to make it easier to improve on this later. The DSI pulse mode has only been tested with a fixed-resolution panel, which limits the testing of different modes on DSI pulse mode. However, as the VSDly calculation also affects pulse mode, so this might cause a regression. Signed-off-by: Tomi Valkeinen --- drivers/gpu/drm/bridge/tc358768.c | 211 +- 1 file changed, 183 insertions(+), 28 deletions(-) diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c index dc2241c18dde..ea19de5509ed 100644 --- a/drivers/gpu/drm/bridge/tc358768.c +++ b/drivers/gpu/drm/bridge/tc358768.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -157,6 +158,7 @@ struct tc358768_priv { u32 frs;/* PLL Freqency range for HSCK (post divider) */ u32 dsiclk; /* pll_clk / 2 */ + u32 pclk; /* incoming pclk rate */ }; static inline struct tc358768_priv *dsi_host_to_tc358768(struct mipi_dsi_host @@ -380,6 +382,7 @@ static int tc358768_calc_pll(struct tc358768_priv *priv, priv->prd = best_prd; priv->frs = frs; priv->dsiclk = best_pll / 2; + priv->pclk = mode->clock * 1000; return 0; } @@ -638,6 +641,28 @@ static u32 tc358768_ps_to_ns(u32 ps) return ps / 1000; } +static u32 tc358768_dpi_to_ns(u32 val, u32 pclk) +{ + return (u32)div_u64((u64)val * NANO, pclk); +} + +/* Convert value in DPI pixel clock units to DSI byte count */ +static u32 tc358768_dpi_to_dsi_bytes(struct tc358768_priv *priv, u32 val) +{ + u64 m = (u64)val * priv->dsiclk / 4 * priv->dsi_lanes; + u64 n = priv->pclk; + + return (u32)div_u64(m + n - 1, n); +} + +static u32 tc358768_dsi_bytes_to_ns(struct tc358768_priv *priv, u32 val) +{ + u64 m = (u64)val * NANO; + u64 n = priv->dsiclk / 4 * priv->dsi_lanes; + + return (u32)div_u64(m, n); +} + static void tc358768_bridge_pre_enable(struct drm_bridge *bridge) { struct tc358768_priv *priv = bridge_to_tc358768(bridge); @@ -647,11 +672,19 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge) s32 raw_val; const struct drm_display_mode *mode; u32 hsbyteclk_ps, dsiclk_ps, ui_ps; - u32 dsiclk, hsbyteclk, video_start; - const u32 internal_delay = 40; + u32 dsiclk, hsbyteclk; int ret, i; struct videomode vm; struct device *dev = priv->dev; + /* In pixelclock units */ + u32 dpi_htot, dpi_data_start; + /* In byte units */ + u32 dsi_dpi_htot, dsi_dpi_data_start; + u32 dsi_hsw, dsi_hbp, dsi_hact, dsi_hfp; + const u32 dsi_hss = 4; /* HSS is a short packet (4 bytes) */ + /* In hsbyteclk units */ + u32 dsi_vsdly; + const u32 internal_dly = 40; if (mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) { dev_warn_once(dev, "Non-continuous mode unimplemented, falling back to continuous\n"); @@ -686,27 +719,23 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge) case MIPI_DSI_FMT_RGB888: val |= (0x3 << 4); hact = vm.hactive * 3; - video_start = (vm.hsync_len + vm.hback_porch) * 3; data_type = MIPI_DSI_PACKED_PIXEL_STREAM_24; break; case MIPI_DSI_FMT_RGB666: val |= (0x4 << 4); hact = vm.hactive * 3; - video_start = (vm.hsync_len + vm.hback_porch) * 3; data_type = MIPI_DSI_PACKED_PIXEL_STREAM_18; break; case MIPI_DSI_FMT_RGB666_PACKED: val |= (0x4 << 4) | BIT(3); hact = vm.hactive * 18 / 8; - video_start = (vm.hsync_len + vm.hback_porch) * 18 / 8; data_type = MIPI_DSI_PIXEL_STREAM_3BYTE_18; break; case MIPI_DSI_FMT_RGB565: val |= (0x5 << 4); hact = vm.hactive * 2; - video_start = (vm.hsync_len + vm.hback_porch) * 2; data_type = MIPI_DSI_PACKED_PIXEL_STREAM_16; break; default: @@ -716,9 +745,150 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge) return; } + /* +* There are three important things to make TC358768
[PATCH v2 05/12] drm/bridge: tc358768: Use struct videomode
The TC358768 documentation uses HFP, HBP, etc. values to deal with the video mode, while the driver currently uses the DRM display mode (htotal, hsync_start, etc). Change the driver to convert the DRM display mode to struct videomode, which then allows us to use the same units the documentation uses. This makes it much easier to work on the code when using the TC358768 documentation as a reference. Signed-off-by: Tomi Valkeinen --- drivers/gpu/drm/bridge/tc358768.c | 45 +-- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c index d5831a1236e9..9b633038af33 100644 --- a/drivers/gpu/drm/bridge/tc358768.c +++ b/drivers/gpu/drm/bridge/tc358768.c @@ -650,6 +650,7 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge) u32 dsiclk, dsibclk, video_start; const u32 internal_delay = 40; int ret, i; + struct videomode vm; if (mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) { dev_warn_once(priv->dev, "Non-continuous mode unimplemented, falling back to continuous\n"); @@ -673,6 +674,8 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge) return; } + drm_display_mode_to_videomode(mode, ); + dsiclk = priv->dsiclk; dsibclk = dsiclk / 4; @@ -681,28 +684,28 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge) switch (dsi_dev->format) { case MIPI_DSI_FMT_RGB888: val |= (0x3 << 4); - hact = mode->hdisplay * 3; - video_start = (mode->htotal - mode->hsync_start) * 3; + hact = vm.hactive * 3; + video_start = (vm.hsync_len + vm.hback_porch) * 3; data_type = MIPI_DSI_PACKED_PIXEL_STREAM_24; break; case MIPI_DSI_FMT_RGB666: val |= (0x4 << 4); - hact = mode->hdisplay * 3; - video_start = (mode->htotal - mode->hsync_start) * 3; + hact = vm.hactive * 3; + video_start = (vm.hsync_len + vm.hback_porch) * 3; data_type = MIPI_DSI_PACKED_PIXEL_STREAM_18; break; case MIPI_DSI_FMT_RGB666_PACKED: val |= (0x4 << 4) | BIT(3); - hact = mode->hdisplay * 18 / 8; - video_start = (mode->htotal - mode->hsync_start) * 18 / 8; + hact = vm.hactive * 18 / 8; + video_start = (vm.hsync_len + vm.hback_porch) * 18 / 8; data_type = MIPI_DSI_PIXEL_STREAM_3BYTE_18; break; case MIPI_DSI_FMT_RGB565: val |= (0x5 << 4); - hact = mode->hdisplay * 2; - video_start = (mode->htotal - mode->hsync_start) * 2; + hact = vm.hactive * 2; + video_start = (vm.hsync_len + vm.hback_porch) * 2; data_type = MIPI_DSI_PACKED_PIXEL_STREAM_16; break; default: @@ -814,43 +817,43 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge) tc358768_write(priv, TC358768_DSI_EVENT, 0); /* vact */ - tc358768_write(priv, TC358768_DSI_VACT, mode->vdisplay); + tc358768_write(priv, TC358768_DSI_VACT, vm.vactive); /* vsw */ - tc358768_write(priv, TC358768_DSI_VSW, - mode->vsync_end - mode->vsync_start); + tc358768_write(priv, TC358768_DSI_VSW, vm.vsync_len); + /* vbp */ - tc358768_write(priv, TC358768_DSI_VBPR, - mode->vtotal - mode->vsync_end); + tc358768_write(priv, TC358768_DSI_VBPR, vm.vback_porch); /* hsw * byteclk * ndl / pclk */ - val = (u32)div_u64((mode->hsync_end - mode->hsync_start) * + val = (u32)div_u64(vm.hsync_len * ((u64)priv->dsiclk / 4) * priv->dsi_lanes, - mode->clock * 1000); + vm.pixelclock); tc358768_write(priv, TC358768_DSI_HSW, val); /* hbp * byteclk * ndl / pclk */ - val = (u32)div_u64((mode->htotal - mode->hsync_end) * + val = (u32)div_u64(vm.hback_porch * ((u64)priv->dsiclk / 4) * priv->dsi_lanes, - mode->clock * 1000); + vm.pixelclock); tc358768_write(priv, TC358768_DSI_HBPR, val); } else { /* Set event mode */ tc358768_write(priv, TC358768_DSI_EVENT, 1); /* vact */ - tc358768_write(priv, TC358768_DSI_VACT, mode->vdisplay); + tc358768_write(priv, TC358768_DSI_VACT, vm.vactive); /* vsw (+ vbp) */
[PATCH v2 08/12] drm/bridge: tc358768: Rename dsibclk to hsbyteclk
The Toshiba documentation talks about HSByteClk when referring to the DSI HS byte clock, whereas the driver uses 'dsibclk' name. Also, in a few places the driver calculates the byte clock from the DSI clock, even if the byte clock is already available in a variable. To align the driver with the documentation, change the 'dsibclk' variable to 'hsbyteclk'. This also make it easier to visually separate 'dsibclk' and 'dsiclk' variables. Signed-off-by: Tomi Valkeinen --- drivers/gpu/drm/bridge/tc358768.c | 48 +++ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c index 3266c08d9bf1..db45b4a982c0 100644 --- a/drivers/gpu/drm/bridge/tc358768.c +++ b/drivers/gpu/drm/bridge/tc358768.c @@ -604,7 +604,7 @@ static int tc358768_setup_pll(struct tc358768_priv *priv, dev_dbg(priv->dev, "PLL: refclk %lu, fbd %u, prd %u, frs %u\n", clk_get_rate(priv->refclk), fbd, prd, frs); - dev_dbg(priv->dev, "PLL: pll_clk: %u, DSIClk %u, DSIByteClk %u\n", + dev_dbg(priv->dev, "PLL: pll_clk: %u, DSIClk %u, HSByteClk %u\n", priv->dsiclk * 2, priv->dsiclk, priv->dsiclk / 4); dev_dbg(priv->dev, "PLL: pclk %u (panel: %u)\n", tc358768_pll_to_pclk(priv, priv->dsiclk * 2), @@ -646,8 +646,8 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge) u32 val, val2, lptxcnt, hact, data_type; s32 raw_val; const struct drm_display_mode *mode; - u32 dsibclk_nsk, dsiclk_nsk, ui_nsk; - u32 dsiclk, dsibclk, video_start; + u32 hsbyteclk_nsk, dsiclk_nsk, ui_nsk; + u32 dsiclk, hsbyteclk, video_start; const u32 internal_delay = 40; int ret, i; struct videomode vm; @@ -678,7 +678,7 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge) drm_display_mode_to_videomode(mode, ); dsiclk = priv->dsiclk; - dsibclk = dsiclk / 4; + hsbyteclk = dsiclk / 4; /* Data Format Control Register */ val = BIT(2) | BIT(1) | BIT(0); /* rdswap_en | dsitx_en | txdt_en */ @@ -730,67 +730,67 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge) tc358768_write(priv, TC358768_D0W_CNTRL + i * 4, 0x); /* DSI Timings */ - dsibclk_nsk = (u32)div_u64((u64)10 * TC358768_PRECISION, - dsibclk); + hsbyteclk_nsk = (u32)div_u64((u64)10 * TC358768_PRECISION, + hsbyteclk); dsiclk_nsk = (u32)div_u64((u64)10 * TC358768_PRECISION, dsiclk); ui_nsk = dsiclk_nsk / 2; dev_dbg(dev, "dsiclk_nsk: %u\n", dsiclk_nsk); dev_dbg(dev, "ui_nsk: %u\n", ui_nsk); - dev_dbg(dev, "dsibclk_nsk: %u\n", dsibclk_nsk); + dev_dbg(dev, "hsbyteclk_nsk: %u\n", hsbyteclk_nsk); /* LP11 > 100us for D-PHY Rx Init */ - val = tc358768_ns_to_cnt(100 * 1000, dsibclk_nsk) - 1; + val = tc358768_ns_to_cnt(100 * 1000, hsbyteclk_nsk) - 1; dev_dbg(dev, "LINEINITCNT: %u\n", val); tc358768_write(priv, TC358768_LINEINITCNT, val); /* LPTimeCnt > 50ns */ - val = tc358768_ns_to_cnt(50, dsibclk_nsk) - 1; + val = tc358768_ns_to_cnt(50, hsbyteclk_nsk) - 1; lptxcnt = val; dev_dbg(dev, "LPTXTIMECNT: %u\n", val); tc358768_write(priv, TC358768_LPTXTIMECNT, val); /* 38ns < TCLK_PREPARE < 95ns */ - val = tc358768_ns_to_cnt(65, dsibclk_nsk) - 1; + val = tc358768_ns_to_cnt(65, hsbyteclk_nsk) - 1; dev_dbg(dev, "TCLK_PREPARECNT %u\n", val); /* TCLK_PREPARE + TCLK_ZERO > 300ns */ val2 = tc358768_ns_to_cnt(300 - tc358768_to_ns(2 * ui_nsk), - dsibclk_nsk) - 2; + hsbyteclk_nsk) - 2; dev_dbg(dev, "TCLK_ZEROCNT %u\n", val2); val |= val2 << 8; tc358768_write(priv, TC358768_TCLK_HEADERCNT, val); /* TCLK_TRAIL > 60ns AND TEOT <= 105 ns + 12*UI */ - raw_val = tc358768_ns_to_cnt(60 + tc358768_to_ns(2 * ui_nsk), dsibclk_nsk) - 5; + raw_val = tc358768_ns_to_cnt(60 + tc358768_to_ns(2 * ui_nsk), hsbyteclk_nsk) - 5; val = clamp(raw_val, 0, 127); dev_dbg(dev, "TCLK_TRAILCNT: %u\n", val); tc358768_write(priv, TC358768_TCLK_TRAILCNT, val); /* 40ns + 4*UI < THS_PREPARE < 85ns + 6*UI */ val = 50 + tc358768_to_ns(4 * ui_nsk); - val = tc358768_ns_to_cnt(val, dsibclk_nsk) - 1; + val = tc358768_ns_to_cnt(val, hsbyteclk_nsk) - 1; dev_dbg(dev, "THS_PREPARECNT %u\n", val); /* THS_PREPARE + THS_ZERO > 145ns + 10*UI */ - raw_val = tc358768_ns_to_cnt(145 - tc358768_to_ns(3 * ui_nsk), dsibclk_nsk) - 10; + raw_val = tc358768_ns_to_cnt(145 - tc358768_to_ns(3 * ui_nsk), hsbyteclk_nsk) - 10; val2 = clamp(raw_val, 0, 127);
[PATCH v2 04/12] drm/bridge: tc358768: Cleanup PLL calculations
As is quite common, some of TC358768's PLL register fields are to be programmed with (value - 1). Specifically, the FBD and PRD, multiplier and divider, are such fields. However, what the driver currently does is that it considers that the formula used for PLL rate calculation is: RefClk * [(FBD + 1)/ (PRD + 1)] * [1 / (2^FRS)] where FBD and PRD are values directly from the registers, while a more sensible way to look at it is: RefClk * FBD / PRD * (1 / (2^FRS)) and when the FBD and PRD values are written to the registers, they will be subtracted by one. Change the driver accordingly, as it simplifies the PLL code. Signed-off-by: Tomi Valkeinen --- drivers/gpu/drm/bridge/tc358768.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c index b668f77673c3..d5831a1236e9 100644 --- a/drivers/gpu/drm/bridge/tc358768.c +++ b/drivers/gpu/drm/bridge/tc358768.c @@ -316,7 +316,7 @@ static int tc358768_calc_pll(struct tc358768_priv *priv, target_pll = tc358768_pclk_to_pll(priv, mode->clock * 1000); - /* pll_clk = RefClk * [(FBD + 1)/ (PRD + 1)] * [1 / (2^FRS)] */ + /* pll_clk = RefClk * FBD / PRD * (1 / (2^FRS)) */ for (i = 0; i < ARRAY_SIZE(frs_limits); i++) if (target_pll >= frs_limits[i]) @@ -336,19 +336,19 @@ static int tc358768_calc_pll(struct tc358768_priv *priv, best_prd = 0; best_fbd = 0; - for (prd = 0; prd < 16; ++prd) { - u32 divisor = (prd + 1) * (1 << frs); + for (prd = 1; prd <= 16; ++prd) { + u32 divisor = prd * (1 << frs); u32 fbd; - for (fbd = 0; fbd < 512; ++fbd) { + for (fbd = 1; fbd <= 512; ++fbd) { u32 pll, diff, pll_in; - pll = (u32)div_u64((u64)refclk * (fbd + 1), divisor); + pll = (u32)div_u64((u64)refclk * fbd, divisor); if (pll >= max_pll || pll < min_pll) continue; - pll_in = (u32)div_u64((u64)refclk, prd + 1); + pll_in = (u32)div_u64((u64)refclk, prd); if (pll_in < 400) continue; @@ -611,7 +611,7 @@ static int tc358768_setup_pll(struct tc358768_priv *priv, mode->clock * 1000); /* PRD[15:12] FBD[8:0] */ - tc358768_write(priv, TC358768_PLLCTL0, (prd << 12) | fbd); + tc358768_write(priv, TC358768_PLLCTL0, ((prd - 1) << 12) | (fbd - 1)); /* FRS[11:10] LBWS[9:8] CKEN[4] RESETB[1] EN[0] */ tc358768_write(priv, TC358768_PLLCTL1, -- 2.34.1
[PATCH v2 06/12] drm/bridge: tc358768: Print logical values, not raw register values
The driver debug prints DSI related timings as raw register values in hex. It is much more useful to see the "logical" value of the timing, not the register value. Change the prints to print the values separately, in case a single register contains multiple values, and use %u to have it in a more human consumable form. Signed-off-by: Tomi Valkeinen --- drivers/gpu/drm/bridge/tc358768.c | 21 - 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c index 9b633038af33..0ef51d04bb21 100644 --- a/drivers/gpu/drm/bridge/tc358768.c +++ b/drivers/gpu/drm/bridge/tc358768.c @@ -739,57 +739,59 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge) /* LP11 > 100us for D-PHY Rx Init */ val = tc358768_ns_to_cnt(100 * 1000, dsibclk_nsk) - 1; - dev_dbg(priv->dev, "LINEINITCNT: 0x%x\n", val); + dev_dbg(priv->dev, "LINEINITCNT: %u\n", val); tc358768_write(priv, TC358768_LINEINITCNT, val); /* LPTimeCnt > 50ns */ val = tc358768_ns_to_cnt(50, dsibclk_nsk) - 1; lptxcnt = val; - dev_dbg(priv->dev, "LPTXTIMECNT: 0x%x\n", val); + dev_dbg(priv->dev, "LPTXTIMECNT: %u\n", val); tc358768_write(priv, TC358768_LPTXTIMECNT, val); /* 38ns < TCLK_PREPARE < 95ns */ val = tc358768_ns_to_cnt(65, dsibclk_nsk) - 1; + dev_dbg(priv->dev, "TCLK_PREPARECNT %u\n", val); /* TCLK_PREPARE + TCLK_ZERO > 300ns */ val2 = tc358768_ns_to_cnt(300 - tc358768_to_ns(2 * ui_nsk), dsibclk_nsk) - 2; + dev_dbg(priv->dev, "TCLK_ZEROCNT %u\n", val2); val |= val2 << 8; - dev_dbg(priv->dev, "TCLK_HEADERCNT: 0x%x\n", val); tc358768_write(priv, TC358768_TCLK_HEADERCNT, val); /* TCLK_TRAIL > 60ns AND TEOT <= 105 ns + 12*UI */ raw_val = tc358768_ns_to_cnt(60 + tc358768_to_ns(2 * ui_nsk), dsibclk_nsk) - 5; val = clamp(raw_val, 0, 127); - dev_dbg(priv->dev, "TCLK_TRAILCNT: 0x%x\n", val); + dev_dbg(priv->dev, "TCLK_TRAILCNT: %u\n", val); tc358768_write(priv, TC358768_TCLK_TRAILCNT, val); /* 40ns + 4*UI < THS_PREPARE < 85ns + 6*UI */ val = 50 + tc358768_to_ns(4 * ui_nsk); val = tc358768_ns_to_cnt(val, dsibclk_nsk) - 1; + dev_dbg(priv->dev, "THS_PREPARECNT %u\n", val); /* THS_PREPARE + THS_ZERO > 145ns + 10*UI */ raw_val = tc358768_ns_to_cnt(145 - tc358768_to_ns(3 * ui_nsk), dsibclk_nsk) - 10; val2 = clamp(raw_val, 0, 127); + dev_dbg(priv->dev, "THS_ZEROCNT %u\n", val2); val |= val2 << 8; - dev_dbg(priv->dev, "THS_HEADERCNT: 0x%x\n", val); tc358768_write(priv, TC358768_THS_HEADERCNT, val); /* TWAKEUP > 1ms in lptxcnt steps */ val = tc358768_ns_to_cnt(102, dsibclk_nsk); val = val / (lptxcnt + 1) - 1; - dev_dbg(priv->dev, "TWAKEUP: 0x%x\n", val); + dev_dbg(priv->dev, "TWAKEUP: %u\n", val); tc358768_write(priv, TC358768_TWAKEUP, val); /* TCLK_POSTCNT > 60ns + 52*UI */ val = tc358768_ns_to_cnt(60 + tc358768_to_ns(52 * ui_nsk), dsibclk_nsk) - 3; - dev_dbg(priv->dev, "TCLK_POSTCNT: 0x%x\n", val); + dev_dbg(priv->dev, "TCLK_POSTCNT: %u\n", val); tc358768_write(priv, TC358768_TCLK_POSTCNT, val); /* max(60ns + 4*UI, 8*UI) < THS_TRAILCNT < 105ns + 12*UI */ raw_val = tc358768_ns_to_cnt(60 + tc358768_to_ns(18 * ui_nsk), dsibclk_nsk) - 4; val = clamp(raw_val, 0, 15); - dev_dbg(priv->dev, "THS_TRAILCNT: 0x%x\n", val); + dev_dbg(priv->dev, "THS_TRAILCNT: %u\n", val); tc358768_write(priv, TC358768_THS_TRAILCNT, val); val = BIT(0); @@ -803,10 +805,11 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge) /* TXTAGOCNT[26:16] RXTASURECNT[10:0] */ val = tc358768_to_ns((lptxcnt + 1) * dsibclk_nsk * 4); val = tc358768_ns_to_cnt(val, dsibclk_nsk) / 4 - 1; + dev_dbg(priv->dev, "TXTAGOCNT: %u\n", val); val2 = tc358768_ns_to_cnt(tc358768_to_ns((lptxcnt + 1) * dsibclk_nsk), dsibclk_nsk) - 2; + dev_dbg(priv->dev, "RXTASURECNT: %u\n", val2); val = val << 16 | val2; - dev_dbg(priv->dev, "BTACNTRL1: 0x%x\n", val); tc358768_write(priv, TC358768_BTACNTRL1, val); /* START[0] */ -- 2.34.1
[PATCH v2 07/12] drm/bridge: tc358768: Use dev for dbg prints, not priv->dev
Simplify the code by capturing the priv->dev value to dev variable, and use it. Signed-off-by: Tomi Valkeinen --- drivers/gpu/drm/bridge/tc358768.c | 41 --- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c index 0ef51d04bb21..3266c08d9bf1 100644 --- a/drivers/gpu/drm/bridge/tc358768.c +++ b/drivers/gpu/drm/bridge/tc358768.c @@ -651,9 +651,10 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge) const u32 internal_delay = 40; int ret, i; struct videomode vm; + struct device *dev = priv->dev; if (mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) { - dev_warn_once(priv->dev, "Non-continuous mode unimplemented, falling back to continuous\n"); + dev_warn_once(dev, "Non-continuous mode unimplemented, falling back to continuous\n"); mode_flags &= ~MIPI_DSI_CLOCK_NON_CONTINUOUS; } @@ -661,7 +662,7 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge) ret = tc358768_sw_reset(priv); if (ret) { - dev_err(priv->dev, "Software reset failed: %d\n", ret); + dev_err(dev, "Software reset failed: %d\n", ret); tc358768_hw_disable(priv); return; } @@ -669,7 +670,7 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge) mode = >encoder->crtc->state->adjusted_mode; ret = tc358768_setup_pll(priv, mode); if (ret) { - dev_err(priv->dev, "PLL setup failed: %d\n", ret); + dev_err(dev, "PLL setup failed: %d\n", ret); tc358768_hw_disable(priv); return; } @@ -709,7 +710,7 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge) data_type = MIPI_DSI_PACKED_PIXEL_STREAM_16; break; default: - dev_err(priv->dev, "Invalid data format (%u)\n", + dev_err(dev, "Invalid data format (%u)\n", dsi_dev->format); tc358768_hw_disable(priv); return; @@ -733,65 +734,65 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge) dsibclk); dsiclk_nsk = (u32)div_u64((u64)10 * TC358768_PRECISION, dsiclk); ui_nsk = dsiclk_nsk / 2; - dev_dbg(priv->dev, "dsiclk_nsk: %u\n", dsiclk_nsk); - dev_dbg(priv->dev, "ui_nsk: %u\n", ui_nsk); - dev_dbg(priv->dev, "dsibclk_nsk: %u\n", dsibclk_nsk); + dev_dbg(dev, "dsiclk_nsk: %u\n", dsiclk_nsk); + dev_dbg(dev, "ui_nsk: %u\n", ui_nsk); + dev_dbg(dev, "dsibclk_nsk: %u\n", dsibclk_nsk); /* LP11 > 100us for D-PHY Rx Init */ val = tc358768_ns_to_cnt(100 * 1000, dsibclk_nsk) - 1; - dev_dbg(priv->dev, "LINEINITCNT: %u\n", val); + dev_dbg(dev, "LINEINITCNT: %u\n", val); tc358768_write(priv, TC358768_LINEINITCNT, val); /* LPTimeCnt > 50ns */ val = tc358768_ns_to_cnt(50, dsibclk_nsk) - 1; lptxcnt = val; - dev_dbg(priv->dev, "LPTXTIMECNT: %u\n", val); + dev_dbg(dev, "LPTXTIMECNT: %u\n", val); tc358768_write(priv, TC358768_LPTXTIMECNT, val); /* 38ns < TCLK_PREPARE < 95ns */ val = tc358768_ns_to_cnt(65, dsibclk_nsk) - 1; - dev_dbg(priv->dev, "TCLK_PREPARECNT %u\n", val); + dev_dbg(dev, "TCLK_PREPARECNT %u\n", val); /* TCLK_PREPARE + TCLK_ZERO > 300ns */ val2 = tc358768_ns_to_cnt(300 - tc358768_to_ns(2 * ui_nsk), dsibclk_nsk) - 2; - dev_dbg(priv->dev, "TCLK_ZEROCNT %u\n", val2); + dev_dbg(dev, "TCLK_ZEROCNT %u\n", val2); val |= val2 << 8; tc358768_write(priv, TC358768_TCLK_HEADERCNT, val); /* TCLK_TRAIL > 60ns AND TEOT <= 105 ns + 12*UI */ raw_val = tc358768_ns_to_cnt(60 + tc358768_to_ns(2 * ui_nsk), dsibclk_nsk) - 5; val = clamp(raw_val, 0, 127); - dev_dbg(priv->dev, "TCLK_TRAILCNT: %u\n", val); + dev_dbg(dev, "TCLK_TRAILCNT: %u\n", val); tc358768_write(priv, TC358768_TCLK_TRAILCNT, val); /* 40ns + 4*UI < THS_PREPARE < 85ns + 6*UI */ val = 50 + tc358768_to_ns(4 * ui_nsk); val = tc358768_ns_to_cnt(val, dsibclk_nsk) - 1; - dev_dbg(priv->dev, "THS_PREPARECNT %u\n", val); + dev_dbg(dev, "THS_PREPARECNT %u\n", val); /* THS_PREPARE + THS_ZERO > 145ns + 10*UI */ raw_val = tc358768_ns_to_cnt(145 - tc358768_to_ns(3 * ui_nsk), dsibclk_nsk) - 10; val2 = clamp(raw_val, 0, 127); - dev_dbg(priv->dev, "THS_ZEROCNT %u\n", val2); + dev_dbg(dev, "THS_ZEROCNT %u\n", val2); val |= val2 << 8; tc358768_write(priv, TC358768_THS_HEADERCNT, val); /* TWAKEUP > 1ms in lptxcnt steps */ val = tc358768_ns_to_cnt(102, dsibclk_nsk); val = val /
[PATCH v2 03/12] drm/bridge: tc358768: Fix bit updates
The driver has a few places where it does: if (thing_is_enabled_in_config) update_thing_bit_in_hw() This means that if the thing is _not_ enabled, the bit never gets cleared. This affects the h/vsyncs and continuous DSI clock bits. Fix the driver to always update the bit. Fixes: ff1ca6397b1d ("drm/bridge: Add tc358768 driver") Signed-off-by: Tomi Valkeinen --- drivers/gpu/drm/bridge/tc358768.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c index bc97a837955b..b668f77673c3 100644 --- a/drivers/gpu/drm/bridge/tc358768.c +++ b/drivers/gpu/drm/bridge/tc358768.c @@ -794,8 +794,8 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge) val |= BIT(i + 1); tc358768_write(priv, TC358768_HSTXVREGEN, val); - if (!(mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS)) - tc358768_write(priv, TC358768_TXOPTIONCNTRL, 0x1); + tc358768_write(priv, TC358768_TXOPTIONCNTRL, + (mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) ? 0 : BIT(0)); /* TXTAGOCNT[26:16] RXTASURECNT[10:0] */ val = tc358768_to_ns((lptxcnt + 1) * dsibclk_nsk * 4); @@ -861,11 +861,12 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge) tc358768_write(priv, TC358768_DSI_HACT, hact); /* VSYNC polarity */ - if (!(mode->flags & DRM_MODE_FLAG_NVSYNC)) - tc358768_update_bits(priv, TC358768_CONFCTL, BIT(5), BIT(5)); + tc358768_update_bits(priv, TC358768_CONFCTL, BIT(5), +(mode->flags & DRM_MODE_FLAG_PVSYNC) ? BIT(5) : 0); + /* HSYNC polarity */ - if (mode->flags & DRM_MODE_FLAG_PHSYNC) - tc358768_update_bits(priv, TC358768_PP_MISC, BIT(0), BIT(0)); + tc358768_update_bits(priv, TC358768_PP_MISC, BIT(0), +(mode->flags & DRM_MODE_FLAG_PHSYNC) ? BIT(0) : 0); /* Start DSI Tx */ tc358768_write(priv, TC358768_DSI_START, 0x1); -- 2.34.1
[PATCH v2 02/12] drm/bridge: tc358768: Fix use of uninitialized variable
smatch reports: drivers/gpu/drm/bridge/tc358768.c:223 tc358768_update_bits() error: uninitialized symbol 'orig'. Fix this by bailing out from tc358768_update_bits() if the tc358768_read() produces an error. Fixes: ff1ca6397b1d ("drm/bridge: Add tc358768 driver") Signed-off-by: Tomi Valkeinen --- drivers/gpu/drm/bridge/tc358768.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c index 819a4b6ec2a0..bc97a837955b 100644 --- a/drivers/gpu/drm/bridge/tc358768.c +++ b/drivers/gpu/drm/bridge/tc358768.c @@ -216,6 +216,10 @@ static void tc358768_update_bits(struct tc358768_priv *priv, u32 reg, u32 mask, u32 tmp, orig; tc358768_read(priv, reg, ); + + if (priv->error) + return; + tmp = orig & ~mask; tmp |= val & mask; if (tmp != orig) -- 2.34.1
[PATCH v2 01/12] drm/tegra: rgb: Parameterize V- and H-sync polarities
From: Thierry Reding The polarities of the V- and H-sync signals are encoded as flags in the display mode, so use the existing information to setup the signals for the RGB interface. Signed-off-by: Thierry Reding Cc: Thierry Reding [tomi.valkei...@ideasonboard.com: default to positive sync] Signed-off-by: Tomi Valkeinen --- drivers/gpu/drm/tegra/rgb.c | 16 +--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/tegra/rgb.c b/drivers/gpu/drm/tegra/rgb.c index 79566c9ea8ff..fc66bbd913b2 100644 --- a/drivers/gpu/drm/tegra/rgb.c +++ b/drivers/gpu/drm/tegra/rgb.c @@ -99,6 +99,7 @@ static void tegra_rgb_encoder_disable(struct drm_encoder *encoder) static void tegra_rgb_encoder_enable(struct drm_encoder *encoder) { + struct drm_display_mode *mode = >crtc->state->adjusted_mode; struct tegra_output *output = encoder_to_output(encoder); struct tegra_rgb *rgb = to_rgb(output); u32 value; @@ -108,10 +109,19 @@ static void tegra_rgb_encoder_enable(struct drm_encoder *encoder) value = DE_SELECT_ACTIVE | DE_CONTROL_NORMAL; tegra_dc_writel(rgb->dc, value, DC_DISP_DATA_ENABLE_OPTIONS); - /* XXX: parameterize? */ + /* configure H- and V-sync signal polarities */ value = tegra_dc_readl(rgb->dc, DC_COM_PIN_OUTPUT_POLARITY(1)); - value &= ~LVS_OUTPUT_POLARITY_LOW; - value &= ~LHS_OUTPUT_POLARITY_LOW; + + if (mode->flags & DRM_MODE_FLAG_NHSYNC) + value |= LHS_OUTPUT_POLARITY_LOW; + else + value &= ~LHS_OUTPUT_POLARITY_LOW; + + if (mode->flags & DRM_MODE_FLAG_NVSYNC) + value |= LVS_OUTPUT_POLARITY_LOW; + else + value &= ~LVS_OUTPUT_POLARITY_LOW; + tegra_dc_writel(rgb->dc, value, DC_COM_PIN_OUTPUT_POLARITY(1)); /* XXX: parameterize? */ -- 2.34.1
[PATCH v2 00/12] drm/bridge: tc358768: Fixes and timings improvements
This series contains various fixes and cleanups for TC358768. The target of this work is to get TC358768 working on Toradex's AM62 based board, which has the following display pipeline: AM62 DPI -> TC358768 -> LT8912B -> HDMI connector The main thing the series does is to improve the DSI HSW, HFP and VSDly calculations. Tomi Signed-off-by: Tomi Valkeinen --- Changes in v2: - Add "drm/tegra: rgb: Parameterize V- and H-sync polarities" so that Tegra can configure the polarities correctly. - Add "drm/bridge: tc358768: Default to positive h/v syncs" as we don't (necessarily) have the polarities set in the mode. - Drop "drm/bridge: tc358768: Add DRM_BRIDGE_ATTACH_NO_CONNECTOR support" as it's not needed for DRM_BRIDGE_ATTACH_NO_CONNECTOR support. - Link to v1: https://lore.kernel.org/r/20230804-tc358768-v1-0-1afd44b78...@ideasonboard.com --- Thierry Reding (1): drm/tegra: rgb: Parameterize V- and H-sync polarities Tomi Valkeinen (11): drm/bridge: tc358768: Fix use of uninitialized variable drm/bridge: tc358768: Fix bit updates drm/bridge: tc358768: Cleanup PLL calculations drm/bridge: tc358768: Use struct videomode drm/bridge: tc358768: Print logical values, not raw register values drm/bridge: tc358768: Use dev for dbg prints, not priv->dev drm/bridge: tc358768: Rename dsibclk to hsbyteclk drm/bridge: tc358768: Clean up clock period code drm/bridge: tc358768: Fix tc358768_ns_to_cnt() drm/bridge: tc358768: Attempt to fix DSI horizontal timings drm/bridge: tc358768: Default to positive h/v syncs drivers/gpu/drm/bridge/tc358768.c | 381 -- drivers/gpu/drm/tegra/rgb.c | 16 +- 2 files changed, 295 insertions(+), 102 deletions(-) --- base-commit: 4d49d87b3606369c6e29b9d051892ee1a6fc4e75 change-id: 20230804-tc358768-1b6949ef2e3d Best regards, -- Tomi Valkeinen
[PATCH] drm: renesas: rcar-du: use proper naming for R-Car
Not RCAR, but R-Car. Signed-off-by: Wolfram Sang --- drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.h b/drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.h index f9893d7d6dfc..e9e59c5e70d5 100644 --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.h +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.h @@ -16,7 +16,7 @@ struct rcar_du_format_info; struct rcar_du_group; /* - * The RCAR DU has 8 hardware planes, shared between primary and overlay planes. + * The R-Car DU has 8 hardware planes, shared between primary and overlay planes. * As using overlay planes requires at least one of the CRTCs being enabled, no * more than 7 overlay planes can be available. We thus create 1 primary plane * per CRTC and 7 overlay planes, for a total of up to 9 KMS planes. -- 2.35.1
Re: [PATCH v5 09/17] drm/imagination: Implement power management
Hi Sarah, Le mercredi 16 août 2023 à 09:25 +0100, Sarah Walker a écrit : > Add power management to the driver, using runtime pm. The power off > sequence depends on firmware commands which are not implemented in > this > patch. > > Changes since v4: > - Suspend runtime PM before unplugging device on rmmod > > Changes since v3: > - Don't power device when calling pvr_device_gpu_fini() > - Documentation for pvr_dev->lost has been improved > - pvr_power_init() renamed to pvr_watchdog_init() > - Use drm_dev_{enter,exit} > > Changes since v2: > - Use runtime PM > - Implement watchdog > > Signed-off-by: Sarah Walker > --- > drivers/gpu/drm/imagination/Makefile | 1 + > drivers/gpu/drm/imagination/pvr_device.c | 23 +- > drivers/gpu/drm/imagination/pvr_device.h | 22 ++ > drivers/gpu/drm/imagination/pvr_drv.c | 20 +- > drivers/gpu/drm/imagination/pvr_power.c | 271 > +++ > drivers/gpu/drm/imagination/pvr_power.h | 39 > 6 files changed, 373 insertions(+), 3 deletions(-) > create mode 100644 drivers/gpu/drm/imagination/pvr_power.c > create mode 100644 drivers/gpu/drm/imagination/pvr_power.h > > diff --git a/drivers/gpu/drm/imagination/Makefile > b/drivers/gpu/drm/imagination/Makefile > index 8fcabc1bea36..235e2d329e29 100644 > --- a/drivers/gpu/drm/imagination/Makefile > +++ b/drivers/gpu/drm/imagination/Makefile > @@ -10,6 +10,7 @@ powervr-y := \ > pvr_fw.o \ > pvr_gem.o \ > pvr_mmu.o \ > + pvr_power.o \ > pvr_vm.o > > obj-$(CONFIG_DRM_POWERVR) += powervr.o > diff --git a/drivers/gpu/drm/imagination/pvr_device.c > b/drivers/gpu/drm/imagination/pvr_device.c > index ef8f7a2ff1a9..5dbd05f21238 100644 > --- a/drivers/gpu/drm/imagination/pvr_device.c > +++ b/drivers/gpu/drm/imagination/pvr_device.c > @@ -5,6 +5,7 @@ > #include "pvr_device_info.h" > > #include "pvr_fw.h" > +#include "pvr_power.h" > #include "pvr_rogue_cr_defs.h" > #include "pvr_vm.h" > > @@ -357,6 +358,8 @@ pvr_device_gpu_fini(struct pvr_device *pvr_dev) > int > pvr_device_init(struct pvr_device *pvr_dev) > { > + struct drm_device *drm_dev = from_pvr_device(pvr_dev); > + struct device *dev = drm_dev->dev; > int err; > > /* Enable and initialize clocks required for the device to > operate. */ > @@ -364,13 +367,29 @@ pvr_device_init(struct pvr_device *pvr_dev) > if (err) > return err; > > + /* Explicitly power the GPU so we can access control > registers before the FW is booted. */ > + err = pm_runtime_resume_and_get(dev); > + if (err) > + return err; > + > /* Map the control registers into memory. */ > err = pvr_device_reg_init(pvr_dev); > if (err) > - return err; > + goto err_pm_runtime_put; > > /* Perform GPU-specific initialization steps. */ > - return pvr_device_gpu_init(pvr_dev); > + err = pvr_device_gpu_init(pvr_dev); > + if (err) > + goto err_pm_runtime_put; > + > + pm_runtime_put(dev); > + > + return 0; > + > +err_pm_runtime_put: > + pm_runtime_put_sync_suspend(dev); > + > + return err; > } > > /** > diff --git a/drivers/gpu/drm/imagination/pvr_device.h > b/drivers/gpu/drm/imagination/pvr_device.h > index 990363e433d7..6d58ecfbc838 100644 > --- a/drivers/gpu/drm/imagination/pvr_device.h > +++ b/drivers/gpu/drm/imagination/pvr_device.h > @@ -135,6 +135,28 @@ struct pvr_device { > > /** @fw_dev: Firmware related data. */ > struct pvr_fw_device fw_dev; > + > + struct { > + /** @work: Work item for watchdog callback. */ > + struct delayed_work work; > + > + /** @old_kccb_cmds_executed: KCCB command execution > count at last watchdog poll. */ > + u32 old_kccb_cmds_executed; > + > + /** @kccb_stall_count: Number of watchdog polls KCCB > has been stalled for. */ > + u32 kccb_stall_count; > + } watchdog; > + > + /** > + * @lost: %true if the device has been lost. > + * > + * This variable is set if the device has become > irretrievably unavailable, e.g. if the > + * firmware processor has stopped responding and can not be > revived via a hard reset. > + */ > + bool lost; > + > + /** @sched_wq: Workqueue for schedulers. */ > + struct workqueue_struct *sched_wq; > }; > > /** > diff --git a/drivers/gpu/drm/imagination/pvr_drv.c > b/drivers/gpu/drm/imagination/pvr_drv.c > index 0d51177b506c..0331dc549116 100644 > --- a/drivers/gpu/drm/imagination/pvr_drv.c > +++ b/drivers/gpu/drm/imagination/pvr_drv.c > @@ -4,6 +4,7 @@ > #include "pvr_device.h" > #include "pvr_drv.h" > #include "pvr_gem.h" > +#include "pvr_power.h" > #include "pvr_rogue_defs.h" > #include "pvr_rogue_fwif_client.h" > #include "pvr_rogue_fwif_shared.h" > @@ -1279,9 +1280,16 @@ pvr_probe(struct
[PATCH] drm/ttm/tests: Fix type conversion in ttm_pool_test
Fix a warning about casting an integer of different size in ttm_pool_alloc_basic_dma_addr() subtest. Cast the DMA address to uintptr_t before casting it to a generic pointer. Signed-off-by: Karolina Stolarek Cc: Christian König Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202308150419.pahfwntn-...@intel.com/ --- drivers/gpu/drm/ttm/tests/ttm_pool_test.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/ttm/tests/ttm_pool_test.c b/drivers/gpu/drm/ttm/tests/ttm_pool_test.c index 8d90870fb199..2d9cae8cd984 100644 --- a/drivers/gpu/drm/ttm/tests/ttm_pool_test.c +++ b/drivers/gpu/drm/ttm/tests/ttm_pool_test.c @@ -228,8 +228,8 @@ static void ttm_pool_alloc_basic_dma_addr(struct kunit *test) dma1 = tt->dma_address[0]; dma2 = tt->dma_address[tt->num_pages - 1]; - KUNIT_ASSERT_NOT_NULL(test, (void *)dma1); - KUNIT_ASSERT_NOT_NULL(test, (void *)dma2); + KUNIT_ASSERT_NOT_NULL(test, (void *)(uintptr_t)dma1); + KUNIT_ASSERT_NOT_NULL(test, (void *)(uintptr_t)dma2); ttm_pool_free(pool, tt); ttm_tt_fini(tt); -- 2.25.1
[PATCH v3 28/41] drm: renesas: shmobile: Rename shmob_drm_connector.connector
Rename the "connector" member of the shmob_drm_connector subclass structure to "base", to improve readability. Signed-off-by: Geert Uytterhoeven Reviewed-by: Laurent Pinchart Reviewed-by: Sui Jingfeng --- v3: - No changes, v2: - Add Reviewed-by. --- drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c | 4 ++-- drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c index 84a773a5363035e0..f55b5263e611c782 100644 --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c @@ -560,7 +560,7 @@ int shmob_drm_encoder_create(struct shmob_drm_device *sdev) static inline struct shmob_drm_connector *to_shmob_connector(struct drm_connector *connector) { - return container_of(connector, struct shmob_drm_connector, connector); + return container_of(connector, struct shmob_drm_connector, base); } static int shmob_drm_connector_get_modes(struct drm_connector *connector) @@ -632,7 +632,7 @@ shmob_drm_connector_init(struct shmob_drm_device *sdev, if (!scon) return ERR_PTR(-ENOMEM); - connector = >connector; + connector = >base; scon->encoder = encoder; scon->mode = >pdata->panel.mode; diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.h b/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.h index 79cce0a0ada4cfce..2c6d7541427581a6 100644 --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.h +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.h @@ -33,7 +33,7 @@ struct shmob_drm_crtc { }; struct shmob_drm_connector { - struct drm_connector connector; + struct drm_connector base; struct drm_encoder *encoder; const struct videomode *mode; }; -- 2.34.1
[PATCH v3 17/41] drm: renesas: shmobile: Convert to use devm_request_irq()
Convert to managed IRQ handling, to simplify cleanup. Signed-off-by: Geert Uytterhoeven Reviewed-by: Laurent Pinchart --- v3: - No changes, v2: - Add Reviewed-by. --- drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c index 91daab80b0ede058..381e184abf552c4c 100644 --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c @@ -196,7 +196,6 @@ static int shmob_drm_remove(struct platform_device *pdev) drm_dev_unregister(ddev); drm_kms_helper_poll_fini(ddev); - free_irq(sdev->irq, ddev); drm_dev_put(ddev); return 0; @@ -279,8 +278,8 @@ static int shmob_drm_probe(struct platform_device *pdev) goto err_modeset_cleanup; sdev->irq = ret; - ret = request_irq(sdev->irq, shmob_drm_irq, 0, ddev->driver->name, - ddev); + ret = devm_request_irq(>dev, sdev->irq, shmob_drm_irq, 0, + ddev->driver->name, ddev); if (ret < 0) { dev_err(>dev, "failed to install IRQ handler\n"); goto err_modeset_cleanup; @@ -292,14 +291,12 @@ static int shmob_drm_probe(struct platform_device *pdev) */ ret = drm_dev_register(ddev, 0); if (ret < 0) - goto err_irq_uninstall; + goto err_modeset_cleanup; drm_fbdev_generic_setup(ddev, 16); return 0; -err_irq_uninstall: - free_irq(sdev->irq, ddev); err_modeset_cleanup: drm_kms_helper_poll_fini(ddev); err_free_drm_dev: -- 2.34.1
[PATCH v3 00/41] drm: renesas: shmobile: Atomic conversion + DT support
Hi all, It has been 3 years since the last conversion of a DRM driver to atomic modesetting, so I guess it's time for another one? ;-) Currently, there are two drivers for the LCD controller on Renesas SuperH-based and ARM-based SH-Mobile and R-Mobile SoCs: 1. sh_mobile_lcdcfb, using the fbdev framework, 2. shmob_drm, using the DRM framework. However, only the former driver is used, as all platform support integrates the former. None of these drivers support DT-based systems. This patch series converts the SH-Mobile DRM driver to atomic modesetting, and adds DT support, complemented by the customary set of fixes and improvements. Overview: - Patch 1 adds a separate maintainer entry. - Patch 2 adds DT bindings for the SH-Mobile LCD controller, - Patch 3 adds definitions for RGB666 9:9 media bus formats, - Patches 4-35 contains miscellaneous fixes, improvements, and cleanups for the SH-Mobile DRM driver, - Patches 36-40 convert the SH-Mobile DRM driver to atomic modesetting, - Patch 41 adds DT support to the SH-Mobile DRM driver. To reduce strain on the audience, I have CCed the DT and media people only on the cover letter and the DT resp. media patches. If interested, the full series should be available through lore.kernel.org. Some comments and questions can be found in the individual patches. Changes compared to v2[1]: - Add Reviewed-by. Changes compared to v1[2]: - New patches - "[PATCH v2 01/41] MAINTAINER: Create entry for Renesas SH-Mobile DRM drivers", - "[PATCH v2 18/41] drm: renesas: shmobile: Remove custom plane destroy callback", - Add myself as co-maintainer, - Make fck clock required, - Drop ports description referring to obsolete graph.txt, - Condition ports to compatible strings, - Drop label and status from example, - Add Reviewed-by, - Drop unused MEDIA_BUS_FMT_RGB666_2X9_LE, as requested by Laurent, - Move explicit clock handling to Runtime PM callbacks, - Move devm_pm_runtime_enable() after shmob_drm_setup_clocks(), - Depend on PM, - Split off removal of call to drm_plane_force_disable(), - Select VIDEOMODE_HELPERS, - Keep table instead of replacing it by a switch() statement, - Fix shmob_drm_interface_data.bus_fmt comment, - Drop superfluous blank lines, - Keep initialization of info fields together, - Use shmob_drm_bus_fmts[], - Keep bus format validation at probe time, - Pass plane type to shmob_drm_plane_create() to avoid having to shift all overlay plane indices by one, - Rename primary_plane_funcs to shmob_drm_primary_plane_funcs, - Rename shmob_drm_plane_funcs to shmob_drm_overlay_plane_funcs, - Move shmob_drm_crtc_finish_page_flip() further up, - Inline shmob_drm_crtc_st{art,op}(), - Use devm_drm_of_get_bridge(), - Don't print bridge->of_node on failure, as this field depends on CONFIG_OF. This has been tested on the R-Mobile A1-based Atmark Techno Armadillo-800-EVA development board, using both legacy[3] and DT-based[4] instantiation, with the fbdev-emulated text console and modetest, a.o. modetest -M shmob-drm -s 43:800x480@RG16 -P 33@41:640x320+80+80@RG16 modetest -M shmob-drm -s 43:800x480@RG16 The output of "modetest -M shmob-drm" can be found below[5]. Thanks for your comments! [1] "[PATCH v2 00/41] drm: renesas: shmobile: Atomic conversion + DT support" https://lore.kernel.org/r/cover.1689698048.git.geert+rene...@glider.be [2] "[PATCH 00/39] drm: renesas: shmobile: Atomic conversion + DT support" https://lore.kernel.org/r/cover.1687423204.git.geert+rene...@glider.be/ [3] "[PATCH/RFC] staging: board: armadillo800eva: Add DRM support" https://lore.kernel.org/r/f7874a9da4bcb20fbc9cd133147b67862ebcf0b9.1687418281.git.geert+rene...@glider.be [4] "[PATCH 0/2] ARM: dts: r8a7740/armadillo800eva: Add LCD support" https://lore.kernel.org/r/cover.1687417585.git.geert+rene...@glider.be [5] Encoders: id crtctypepossible crtcs possible clones 42 41 DPI 0x0001 0x0001 Connectors: id encoder status namesize (mm) modes encoders 43 42 connected DPI-1 111x67 1 42 modes: index name refresh (Hz) hdisp hss hse htot vdisp vss vse vtot #0 800x480 59.99 800 840 968 1056 480 515 517 525 33260 flags: nhsync, nvsync; type: preferred, driver props: 1 EDID: flags: immutable blob blobs: value: 2 DPMS: flags: enum enums: On=0 Standby=1 Suspend=2 Off=3 value: 0 5 link-status: flags: enum enums: Good=0 Bad=1 value: 0 6 non-desktop: flags: immutable range values: 0 1 value: 0 4 TILE:
[PATCH v3 12/41] drm: renesas: shmobile: Remove backlight support
From: Laurent Pinchart Backlight support should be implemented by panels, not by the LCDC driver. As the feature is currently unused anyway, remove it. Signed-off-by: Laurent Pinchart [geert: Cleanups] Signed-off-by: Geert Uytterhoeven Reviewed-by: Laurent Pinchart --- v3: - No changes, v2: - Add Reviewed-by. Changes compared to Laurent's original: - Rebase, - Remove unused variable ‘scon’, - Remove now unused to_shmob_encoder() macro, - Remove now empty shmob_drm_encoder wrapper. --- drivers/gpu/drm/renesas/shmobile/Makefile | 3 +- .../renesas/shmobile/shmob_drm_backlight.c| 82 --- .../renesas/shmobile/shmob_drm_backlight.h| 19 - .../gpu/drm/renesas/shmobile/shmob_drm_crtc.c | 33 +--- .../gpu/drm/renesas/shmobile/shmob_drm_crtc.h | 8 -- .../gpu/drm/renesas/shmobile/shmob_drm_drv.h | 2 +- .../gpu/drm/renesas/shmobile/shmob_drm_kms.c | 2 +- include/linux/platform_data/shmob_drm.h | 8 -- 8 files changed, 7 insertions(+), 150 deletions(-) delete mode 100644 drivers/gpu/drm/renesas/shmobile/shmob_drm_backlight.c delete mode 100644 drivers/gpu/drm/renesas/shmobile/shmob_drm_backlight.h diff --git a/drivers/gpu/drm/renesas/shmobile/Makefile b/drivers/gpu/drm/renesas/shmobile/Makefile index 861edafed8562c87..2679555d61a70207 100644 --- a/drivers/gpu/drm/renesas/shmobile/Makefile +++ b/drivers/gpu/drm/renesas/shmobile/Makefile @@ -1,6 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 -shmob-drm-y := shmob_drm_backlight.o \ - shmob_drm_crtc.o \ +shmob-drm-y := shmob_drm_crtc.o \ shmob_drm_drv.o \ shmob_drm_kms.o \ shmob_drm_plane.o diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_backlight.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_backlight.c deleted file mode 100644 index 794573badfe86076.. --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_backlight.c +++ /dev/null @@ -1,82 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0+ -/* - * shmob_drm_backlight.c -- SH Mobile DRM Backlight - * - * Copyright (C) 2012 Renesas Electronics Corporation - * - * Laurent Pinchart (laurent.pinch...@ideasonboard.com) - */ - -#include - -#include "shmob_drm_backlight.h" -#include "shmob_drm_crtc.h" -#include "shmob_drm_drv.h" - -static int shmob_drm_backlight_update(struct backlight_device *bdev) -{ - struct shmob_drm_connector *scon = bl_get_data(bdev); - struct shmob_drm_device *sdev = scon->connector.dev->dev_private; - const struct shmob_drm_backlight_data *bdata = >pdata->backlight; - int brightness = backlight_get_brightness(bdev); - - return bdata->set_brightness(brightness); -} - -static int shmob_drm_backlight_get_brightness(struct backlight_device *bdev) -{ - struct shmob_drm_connector *scon = bl_get_data(bdev); - struct shmob_drm_device *sdev = scon->connector.dev->dev_private; - const struct shmob_drm_backlight_data *bdata = >pdata->backlight; - - return bdata->get_brightness(); -} - -static const struct backlight_ops shmob_drm_backlight_ops = { - .options= BL_CORE_SUSPENDRESUME, - .update_status = shmob_drm_backlight_update, - .get_brightness = shmob_drm_backlight_get_brightness, -}; - -void shmob_drm_backlight_dpms(struct shmob_drm_connector *scon, int mode) -{ - if (scon->backlight == NULL) - return; - - scon->backlight->props.power = mode == DRM_MODE_DPMS_ON -? FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN; - backlight_update_status(scon->backlight); -} - -int shmob_drm_backlight_init(struct shmob_drm_connector *scon) -{ - struct shmob_drm_device *sdev = scon->connector.dev->dev_private; - const struct shmob_drm_backlight_data *bdata = >pdata->backlight; - struct drm_connector *connector = >connector; - struct drm_device *dev = connector->dev; - struct backlight_device *backlight; - - if (!bdata->max_brightness) - return 0; - - backlight = backlight_device_register(bdata->name, dev->dev, scon, - _drm_backlight_ops, NULL); - if (IS_ERR(backlight)) { - dev_err(dev->dev, "unable to register backlight device: %ld\n", - PTR_ERR(backlight)); - return PTR_ERR(backlight); - } - - backlight->props.max_brightness = bdata->max_brightness; - backlight->props.brightness = bdata->max_brightness; - backlight->props.power = FB_BLANK_POWERDOWN; - backlight_update_status(backlight); - - scon->backlight = backlight; - return 0; -} - -void shmob_drm_backlight_exit(struct shmob_drm_connector *scon) -{ - backlight_device_unregister(scon->backlight); -} diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_backlight.h b/drivers/gpu/drm/renesas/shmobile/shmob_drm_backlight.h deleted file mode 100644 index d9abb7a60be5c414..
[PATCH v3 19/41] drm: renesas: shmobile: Use drmm_universal_plane_alloc()
According to the comments for drm_universal_plane_init(), the plane structure should not be allocated with devm_kzalloc(). Fix lifetime issues by using drmm_universal_plane_alloc() instead. Signed-off-by: Geert Uytterhoeven --- v3: - No changes, v2: - Split off removal of call to drm_plane_force_disable(). --- .../gpu/drm/renesas/shmobile/shmob_drm_plane.c | 18 +++--- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c index 3a5db319bad14218..1fb68b5fe915b8dc 100644 --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c @@ -179,7 +179,6 @@ static int shmob_drm_plane_disable(struct drm_plane *plane, static const struct drm_plane_funcs shmob_drm_plane_funcs = { .update_plane = shmob_drm_plane_update, .disable_plane = shmob_drm_plane_disable, - .destroy = drm_plane_cleanup, }; static const uint32_t formats[] = { @@ -198,19 +197,16 @@ static const uint32_t formats[] = { int shmob_drm_plane_create(struct shmob_drm_device *sdev, unsigned int index) { struct shmob_drm_plane *splane; - int ret; - splane = devm_kzalloc(sdev->dev, sizeof(*splane), GFP_KERNEL); - if (splane == NULL) - return -ENOMEM; + splane = drmm_universal_plane_alloc(sdev->ddev, struct shmob_drm_plane, + plane, 1, _drm_plane_funcs, + formats, ARRAY_SIZE(formats), NULL, + DRM_PLANE_TYPE_OVERLAY, NULL); + if (IS_ERR(splane)) + return PTR_ERR(splane); splane->index = index; splane->alpha = 255; - ret = drm_universal_plane_init(sdev->ddev, >plane, 1, - _drm_plane_funcs, - formats, ARRAY_SIZE(formats), NULL, - DRM_PLANE_TYPE_OVERLAY, NULL); - - return ret; + return 0; } -- 2.34.1
[PATCH v3 05/41] drm: renesas: shmobile: Fix ARGB32 overlay format typo
When configuring a CHn Source Image Format Register (LDBBSIFR), one should use the corresponding LDBBSIFR_RPKF_* definition for overlay planes, not the DDFR_PKF_* definition for the primary plane. Fortunately both definitions resolve to the same value, so this bug did not cause any harm. Signed-off-by: Geert Uytterhoeven Reviewed-by: Laurent Pinchart --- v3: - No changes, v2: - s/configurating/configuring/, - Add Reviewed-by. --- drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c index 0e34573c3cb3d032..7e49e2873da1bb6f 100644 --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c @@ -94,10 +94,10 @@ static void __shmob_drm_plane_setup(struct shmob_drm_plane *splane, format |= LDBBSIFR_AL_1 | LDBBSIFR_RY | LDBBSIFR_RPKF_RGB24; break; case DRM_FORMAT_ARGB: - format |= LDBBSIFR_AL_PK | LDBBSIFR_RY | LDDFR_PKF_ARGB32; + format |= LDBBSIFR_AL_PK | LDBBSIFR_RY | LDBBSIFR_RPKF_ARGB32; break; case DRM_FORMAT_XRGB: - format |= LDBBSIFR_AL_1 | LDBBSIFR_RY | LDDFR_PKF_ARGB32; + format |= LDBBSIFR_AL_1 | LDBBSIFR_RY | LDBBSIFR_RPKF_ARGB32; break; case DRM_FORMAT_NV12: case DRM_FORMAT_NV21: -- 2.34.1
[PATCH v3 03/41] media: uapi: Add MEDIA_BUS_FMT_RGB666_2X9_BE format
Add the RGB666 9:9 format MEDIA_BUS_FMT_RGB666_2X9_BE, which is supported by the SH-Mobile LCD Controller. Signed-off-by: Geert Uytterhoeven Reviewed-by: Laurent Pinchart --- Cc: Mauro Carvalho Chehab Cc: Hans Verkuil Cc: linux-me...@vger.kernel.org v3: - No changes, v2: - Add Reviewed-by, - Drop unused MEDIA_BUS_FMT_RGB666_2X9_LE, as requested by Laurent. --- .../media/v4l/subdev-formats.rst | 72 +++ include/uapi/linux/media-bus-format.h | 3 +- 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst index a3a35eeed70846ba..eb3cd20b0cf2e3d6 100644 --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst @@ -949,6 +949,78 @@ The following tables list existing packed RGB formats. - b\ :sub:`2` - b\ :sub:`1` - b\ :sub:`0` +* .. _MEDIA-BUS-FMT-RGB666-2X9-BE: + + - MEDIA_BUS_FMT_RGB666_2X9_BE + - 0x1025 + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - r\ :sub:`5` + - r\ :sub:`4` + - r\ :sub:`3` + - r\ :sub:`2` + - r\ :sub:`1` + - r\ :sub:`0` + - g\ :sub:`5` + - g\ :sub:`4` + - g\ :sub:`3` +* - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - g\ :sub:`2` + - g\ :sub:`1` + - g\ :sub:`0` + - b\ :sub:`5` + - b\ :sub:`4` + - b\ :sub:`3` + - b\ :sub:`2` + - b\ :sub:`1` + - b\ :sub:`0` * .. _MEDIA-BUS-FMT-BGR666-1X18: - MEDIA_BUS_FMT_BGR666_1X18 diff --git a/include/uapi/linux/media-bus-format.h b/include/uapi/linux/media-bus-format.h index a03c543cb072de30..f05f747e444d6686 100644 --- a/include/uapi/linux/media-bus-format.h +++ b/include/uapi/linux/media-bus-format.h @@ -34,7 +34,7 @@ #define MEDIA_BUS_FMT_FIXED0x0001 -/* RGB - next is 0x1025 */ +/* RGB - next is 0x1026 */ #define MEDIA_BUS_FMT_RGB444_1X12 0x1016 #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE 0x1001 #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE 0x1002 @@ -46,6 +46,7 @@ #define MEDIA_BUS_FMT_RGB565_2X8_BE0x1007 #define MEDIA_BUS_FMT_RGB565_2X8_LE0x1008 #define MEDIA_BUS_FMT_RGB666_1X18 0x1009 +#define MEDIA_BUS_FMT_RGB666_2X9_BE0x1025 #define MEDIA_BUS_FMT_BGR666_1X18 0x1023 #define MEDIA_BUS_FMT_RBG888_1X24 0x100e #define MEDIA_BUS_FMT_RGB666_1X24_CPADHI 0x1015 -- 2.34.1
[PATCH v3 27/41] drm: renesas: shmobile: Rename shmob_drm_crtc.crtc
Rename the "crtc" member of the shmob_drm_crtc subclass structure to "base", to improve readability. Signed-off-by: Geert Uytterhoeven Reviewed-by: Laurent Pinchart Reviewed-by: Sui Jingfeng --- v3: - No changes, v2: - Add Reviewed-by. --- .../gpu/drm/renesas/shmobile/shmob_drm_crtc.c | 26 +-- .../gpu/drm/renesas/shmobile/shmob_drm_crtc.h | 2 +- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c index 564051b505ed4ac5..84a773a5363035e0 100644 --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c @@ -55,7 +55,7 @@ static const struct { static void shmob_drm_crtc_setup_geometry(struct shmob_drm_crtc *scrtc) { - struct drm_crtc *crtc = >crtc; + struct drm_crtc *crtc = >base; struct shmob_drm_device *sdev = to_shmob_device(crtc->dev); const struct drm_display_info *info = >connector->display_info; const struct drm_display_mode *mode = >mode; @@ -114,7 +114,7 @@ static void shmob_drm_crtc_setup_geometry(struct shmob_drm_crtc *scrtc) static void shmob_drm_crtc_start_stop(struct shmob_drm_crtc *scrtc, bool start) { - struct shmob_drm_device *sdev = to_shmob_device(scrtc->crtc.dev); + struct shmob_drm_device *sdev = to_shmob_device(scrtc->base.dev); u32 value; value = lcdc_read(sdev, LDCNT2R); @@ -147,7 +147,7 @@ static void shmob_drm_crtc_start_stop(struct shmob_drm_crtc *scrtc, bool start) */ static void shmob_drm_crtc_start(struct shmob_drm_crtc *scrtc) { - struct drm_crtc *crtc = >crtc; + struct drm_crtc *crtc = >base; struct shmob_drm_device *sdev = to_shmob_device(crtc->dev); const struct shmob_drm_interface_data *idata = >pdata->iface; const struct shmob_drm_format_info *format; @@ -227,7 +227,7 @@ static void shmob_drm_crtc_start(struct shmob_drm_crtc *scrtc) static void shmob_drm_crtc_stop(struct shmob_drm_crtc *scrtc) { - struct drm_crtc *crtc = >crtc; + struct drm_crtc *crtc = >base; struct shmob_drm_device *sdev = to_shmob_device(crtc->dev); if (!scrtc->started) @@ -260,7 +260,7 @@ void shmob_drm_crtc_resume(struct shmob_drm_crtc *scrtc) static void shmob_drm_crtc_compute_base(struct shmob_drm_crtc *scrtc, int x, int y) { - struct drm_crtc *crtc = >crtc; + struct drm_crtc *crtc = >base; struct drm_framebuffer *fb = crtc->primary->fb; struct drm_gem_dma_object *gem; unsigned int bpp; @@ -281,7 +281,7 @@ static void shmob_drm_crtc_compute_base(struct shmob_drm_crtc *scrtc, static void shmob_drm_crtc_update_base(struct shmob_drm_crtc *scrtc) { - struct drm_crtc *crtc = >crtc; + struct drm_crtc *crtc = >base; struct shmob_drm_device *sdev = to_shmob_device(crtc->dev); shmob_drm_crtc_compute_base(scrtc, crtc->x, crtc->y); @@ -295,7 +295,7 @@ static void shmob_drm_crtc_update_base(struct shmob_drm_crtc *scrtc) static inline struct shmob_drm_crtc *to_shmob_crtc(struct drm_crtc *crtc) { - return container_of(crtc, struct shmob_drm_crtc, crtc); + return container_of(crtc, struct shmob_drm_crtc, base); } static void shmob_drm_crtc_dpms(struct drm_crtc *crtc, int mode) @@ -367,15 +367,15 @@ static const struct drm_crtc_helper_funcs crtc_helper_funcs = { void shmob_drm_crtc_finish_page_flip(struct shmob_drm_crtc *scrtc) { struct drm_pending_vblank_event *event; - struct drm_device *dev = scrtc->crtc.dev; + struct drm_device *dev = scrtc->base.dev; unsigned long flags; spin_lock_irqsave(>event_lock, flags); event = scrtc->event; scrtc->event = NULL; if (event) { - drm_crtc_send_vblank_event(>crtc, event); - drm_crtc_vblank_put(>crtc); + drm_crtc_send_vblank_event(>base, event); + drm_crtc_vblank_put(>base); } spin_unlock_irqrestore(>event_lock, flags); } @@ -387,7 +387,7 @@ static int shmob_drm_crtc_page_flip(struct drm_crtc *crtc, struct drm_modeset_acquire_ctx *ctx) { struct shmob_drm_crtc *scrtc = to_shmob_crtc(crtc); - struct drm_device *dev = scrtc->crtc.dev; + struct drm_device *dev = scrtc->base.dev; unsigned long flags; spin_lock_irqsave(>event_lock, flags); @@ -402,7 +402,7 @@ static int shmob_drm_crtc_page_flip(struct drm_crtc *crtc, if (event) { event->pipe = 0; - drm_crtc_vblank_get(>crtc); + drm_crtc_vblank_get(>base); spin_lock_irqsave(>event_lock, flags); scrtc->event = event; spin_unlock_irqrestore(>event_lock, flags); @@ -454,7 +454,7 @@ static const struct drm_crtc_funcs crtc_funcs = { int
[PATCH v3 26/41] drm: renesas: shmobile: Unify plane allocation
Unify primary and overlay plane allocation: - Enhance shmob_drm_plane_create() so it can be used to create the primary plane, too, - Move overlay plane creation next to primary plane creation. Signed-off-by: Geert Uytterhoeven --- v3: - No changes, v2: - Pass plane type to shmob_drm_plane_create() to avoid having to shift all overlay plane indices by one, - Rename primary_plane_funcs to shmob_drm_primary_plane_funcs, - Rename shmob_drm_plane_funcs to shmob_drm_overlay_plane_funcs. --- .../gpu/drm/renesas/shmobile/shmob_drm_crtc.c | 39 +-- .../gpu/drm/renesas/shmobile/shmob_drm_drv.c | 9 - .../drm/renesas/shmobile/shmob_drm_plane.c| 31 +++ .../drm/renesas/shmobile/shmob_drm_plane.h| 4 +- 4 files changed, 36 insertions(+), 47 deletions(-) diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c index caafd04acd3d46d0..564051b505ed4ac5 100644 --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c @@ -19,7 +19,6 @@ #include #include #include -#include #include #include #include @@ -453,47 +452,29 @@ static const struct drm_crtc_funcs crtc_funcs = { .disable_vblank = shmob_drm_disable_vblank, }; -static const uint32_t modeset_formats[] = { - DRM_FORMAT_RGB565, - DRM_FORMAT_RGB888, - DRM_FORMAT_ARGB, - DRM_FORMAT_XRGB, - DRM_FORMAT_NV12, - DRM_FORMAT_NV21, - DRM_FORMAT_NV16, - DRM_FORMAT_NV61, - DRM_FORMAT_NV24, - DRM_FORMAT_NV42, -}; - -static const struct drm_plane_funcs primary_plane_funcs = { - DRM_PLANE_NON_ATOMIC_FUNCS, -}; - int shmob_drm_crtc_create(struct shmob_drm_device *sdev) { struct drm_crtc *crtc = >crtc.crtc; - struct drm_plane *primary; + struct drm_plane *primary, *plane; + unsigned int i; int ret; sdev->crtc.dpms = DRM_MODE_DPMS_OFF; - primary = __drm_universal_plane_alloc(>ddev, sizeof(*primary), 0, - 0, _plane_funcs, - modeset_formats, - ARRAY_SIZE(modeset_formats), - NULL, DRM_PLANE_TYPE_PRIMARY, - NULL); + primary = shmob_drm_plane_create(sdev, DRM_PLANE_TYPE_PRIMARY, 0); if (IS_ERR(primary)) return PTR_ERR(primary); + for (i = 1; i < 5; ++i) { + plane = shmob_drm_plane_create(sdev, DRM_PLANE_TYPE_OVERLAY, i); + if (IS_ERR(plane)) + return PTR_ERR(plane); + } + ret = drm_crtc_init_with_planes(>ddev, crtc, primary, NULL, _funcs, NULL); - if (ret < 0) { - drm_plane_cleanup(primary); - kfree(primary); + if (ret < 0) return ret; - } drm_crtc_helper_add(crtc, _helper_funcs); diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c index 8da3054f26b2d235..6bc05a9e9661915e 100644 --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c @@ -181,7 +181,6 @@ static int shmob_drm_probe(struct platform_device *pdev) struct shmob_drm_platform_data *pdata = pdev->dev.platform_data; struct shmob_drm_device *sdev; struct drm_device *ddev; - unsigned int i; int ret; if (pdata == NULL) { @@ -222,14 +221,6 @@ static int shmob_drm_probe(struct platform_device *pdev) return dev_err_probe(>dev, ret, "failed to initialize mode setting\n"); - for (i = 0; i < 4; ++i) { - ret = shmob_drm_plane_create(sdev, i); - if (ret < 0) { - dev_err(>dev, "failed to create plane %u\n", i); - goto err_modeset_cleanup; - } - } - ret = drm_vblank_init(ddev, 1); if (ret < 0) { dev_err(>dev, "failed to initialize vblank\n"); diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c index c58b9dca34736342..3518f8900c0d1f9e 100644 --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c @@ -12,6 +12,7 @@ #include #include #include +#include #include "shmob_drm_drv.h" #include "shmob_drm_kms.h" @@ -179,7 +180,12 @@ static int shmob_drm_plane_disable(struct drm_plane *plane, return 0; } -static const struct drm_plane_funcs shmob_drm_plane_funcs = { +static const struct drm_plane_funcs shmob_drm_primary_plane_funcs = { + .update_plane = drm_plane_helper_update_primary, + .disable_plane =
[PATCH v3 34/41] drm: renesas: shmobile: Shutdown the display on remove
When the device is unbound from the driver, the display may be active. Make sure it gets shut down. Signed-off-by: Geert Uytterhoeven Reviewed-by: Laurent Pinchart --- v3: - No changes, v2: - Add Reviewed-by. --- drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c index 78f9650e3a61365f..51a9a6955d1fb0fb 100644 --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c @@ -16,6 +16,7 @@ #include #include +#include #include #include #include @@ -172,6 +173,7 @@ static int shmob_drm_remove(struct platform_device *pdev) struct drm_device *ddev = >ddev; drm_dev_unregister(ddev); + drm_helper_force_disable_all(ddev); drm_kms_helper_poll_fini(ddev); return 0; } -- 2.34.1
[PATCH v3 02/41] dt-bindings: display: Add Renesas SH-Mobile LCDC bindings
Add device tree bindings for the LCD Controller (LCDC) found in Renesas SuperH SH-Mobile and ARM SH/R-Mobile SOCs. Based on a plain text prototype by Laurent Pinchart. Signed-off-by: Geert Uytterhoeven Reviewed-by: Rob Herring --- Cc: Rob Herring Cc: Krzysztof Kozlowski Cc: Conor Dooley Cc: devicet...@vger.kernel.org v3: - Add Reviewed-by, v2: - Add myself as co-maintainer, - Make fck clock required, - Drop ports description referring to obsolete graph.txt, - Condition ports to compatible strings, - Drop label and status from example. Changes compared to Laurent's original: - Convert to json-schema, - Rename compatible values from "renesas,lcdc-" to "renesas,-lcdc", - Add power-domains property, - Add MIPI-DSI port on SH-Mobile AG5, - Update example to reflect reality, - Add to MAINTAINERS. --- .../display/renesas,shmobile-lcdc.yaml| 130 ++ MAINTAINERS | 1 + 2 files changed, 131 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/renesas,shmobile-lcdc.yaml diff --git a/Documentation/devicetree/bindings/display/renesas,shmobile-lcdc.yaml b/Documentation/devicetree/bindings/display/renesas,shmobile-lcdc.yaml new file mode 100644 index ..9816c4cacc7d9a7f --- /dev/null +++ b/Documentation/devicetree/bindings/display/renesas,shmobile-lcdc.yaml @@ -0,0 +1,130 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/renesas,shmobile-lcdc.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Renesas SH-Mobile LCD Controller (LCDC) + +maintainers: + - Laurent Pinchart + - Geert Uytterhoeven + +properties: + compatible: +enum: + - renesas,r8a7740-lcdc # R-Mobile A1 + - renesas,sh73a0-lcdc # SH-Mobile AG5 + + reg: +maxItems: 1 + + interrupts: +maxItems: 1 + + clocks: +minItems: 1 +maxItems: 5 +description: + Only the functional clock is mandatory. + Some of the optional clocks are model-dependent (e.g. "video" (a.k.a. + "vou" or "dv_clk") is available on R-Mobile A1 only). + + clock-names: +minItems: 1 +items: + - const: fck + - enum: [ media, lclk, hdmi, video ] + - enum: [ media, lclk, hdmi, video ] + - enum: [ media, lclk, hdmi, video ] + - enum: [ media, lclk, hdmi, video ] + + power-domains: +maxItems: 1 + + ports: +$ref: /schemas/graph.yaml#/properties/ports + +properties: + port@0: +$ref: /schemas/graph.yaml#/properties/port +description: LCD port (R-Mobile A1 and SH-Mobile AG5) +unevaluatedProperties: false + + port@1: +$ref: /schemas/graph.yaml#/properties/port +description: HDMI port (R-Mobile A1 LCDC1 and SH-Mobile AG5) +unevaluatedProperties: false + + port@2: +$ref: /schemas/graph.yaml#/properties/port +description: MIPI-DSI port (SH-Mobile AG5) +unevaluatedProperties: false + +required: + - port@0 + +unevaluatedProperties: false + +required: + - compatible + - reg + - interrupts + - clocks + - clock-names + - power-domains + - ports + +additionalProperties: false + +allOf: + - if: + properties: +compatible: + contains: +const: renesas,r8a7740-lcdc +then: + properties: +ports: + properties: +port@2: false + + - if: + properties: +compatible: + contains: +const: renesas,sh73a0-lcdc +then: + properties: +ports: + required: +- port@1 +- port@2 + +examples: + - | +#include +#include + +lcd-controller@fe94 { +compatible = "renesas,r8a7740-lcdc"; +reg = <0xfe94 0x4000>; +interrupts = ; +clocks = <_clks R8A7740_CLK_LCDC0>, + <_clocks R8A7740_CLK_M3>, <_clk>, + <_clk>; +clock-names = "fck", "media", "lclk", "video"; +power-domains = <_a4lc>; + +ports { +#address-cells = <1>; +#size-cells = <0>; + +port@0 { +reg = <0>; + +lcdc0_rgb: endpoint { +}; +}; +}; +}; diff --git a/MAINTAINERS b/MAINTAINERS index c454b0186fd669dd..d3e1b194dfbd129e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7026,6 +7026,7 @@ M:Geert Uytterhoeven L: dri-devel@lists.freedesktop.org L: linux-renesas-...@vger.kernel.org S: Supported +F: Documentation/devicetree/bindings/display/renesas,shmobile-lcdc.yaml F: drivers/gpu/drm/renesas/shmobile/ F: include/linux/platform_data/shmob_drm.h -- 2.34.1
[PATCH v3 18/41] drm: renesas: shmobile: Remove custom plane destroy callback
There is no need to call drm_plane_force_disable() from the plane's .destroy() callback, as the plane should have been disabled already before. See also commit 3c858a33858baa8c ("drm/plane_helper: don't disable plane in destroy function") for the generic plane helper case. After removing this call, shmob_drm_plane_destroy() becomes a simple wrapper around shmob_drm_plane_destroy(), hence replace it by the latter. Signed-off-by: Geert Uytterhoeven --- v3: - No changes, v2: - New. --- drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c index 0b2ab153e9ae76df..3a5db319bad14218 100644 --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c @@ -176,16 +176,10 @@ static int shmob_drm_plane_disable(struct drm_plane *plane, return 0; } -static void shmob_drm_plane_destroy(struct drm_plane *plane) -{ - drm_plane_force_disable(plane); - drm_plane_cleanup(plane); -} - static const struct drm_plane_funcs shmob_drm_plane_funcs = { .update_plane = shmob_drm_plane_update, .disable_plane = shmob_drm_plane_disable, - .destroy = shmob_drm_plane_destroy, + .destroy = drm_plane_cleanup, }; static const uint32_t formats[] = { -- 2.34.1
[PATCH v3 23/41] drm: renesas: shmobile: Use struct videomode in platform data
From: Laurent Pinchart Replace the drm_mode_modeinfo field with videomode that includes more signal polarity flags. This simplifies driver handling of panel modes and prepares for DT support. Signed-off-by: Laurent Pinchart [geert: Simplify] Signed-off-by: Geert Uytterhoeven Reviewed-by: Laurent Pinchart --- v3: - No changes, v2: - Add Reviewed-by, - Select VIDEOMODE_HELPERS. Changes compared to Laurent's original: - Rebase, - Fix build, - Remove unneeded {width,height}_mm intermediaries from shmob_drm_connector, - Replace embedded videomode by a const pointer to pdata. --- drivers/gpu/drm/renesas/shmobile/Kconfig | 1 + .../gpu/drm/renesas/shmobile/shmob_drm_crtc.c | 35 --- .../gpu/drm/renesas/shmobile/shmob_drm_crtc.h | 3 ++ include/linux/platform_data/shmob_drm.h | 11 ++ 4 files changed, 20 insertions(+), 30 deletions(-) diff --git a/drivers/gpu/drm/renesas/shmobile/Kconfig b/drivers/gpu/drm/renesas/shmobile/Kconfig index ba941587ca70e08c..027220b8fe1c5fbd 100644 --- a/drivers/gpu/drm/renesas/shmobile/Kconfig +++ b/drivers/gpu/drm/renesas/shmobile/Kconfig @@ -6,6 +6,7 @@ config DRM_SHMOBILE select BACKLIGHT_CLASS_DEVICE select DRM_KMS_HELPER select DRM_GEM_DMA_HELPER + select VIDEOMODE_HELPERS help Choose this option if you have an SH Mobile chipset. If M is selected the module will be called shmob-drm. diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c index f62ae047a48f615b..b3ef10b7828de197 100644 --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c @@ -23,6 +23,8 @@ #include #include +#include + #include "shmob_drm_crtc.h" #include "shmob_drm_drv.h" #include "shmob_drm_kms.h" @@ -41,18 +43,16 @@ static void shmob_drm_crtc_setup_geometry(struct shmob_drm_crtc *scrtc) { struct drm_crtc *crtc = >crtc; struct shmob_drm_device *sdev = to_shmob_device(crtc->dev); - const struct shmob_drm_interface_data *idata = >pdata->iface; + enum display_flags dpy_flags = sdev->connector.mode->flags; const struct drm_display_mode *mode = >mode; u32 value; value = sdev->ldmt1r | ((mode->flags & DRM_MODE_FLAG_PVSYNC) ? 0 : LDMT1R_VPOL) | ((mode->flags & DRM_MODE_FLAG_PHSYNC) ? 0 : LDMT1R_HPOL) - | ((idata->flags & SHMOB_DRM_IFACE_FL_DWPOL) ? LDMT1R_DWPOL : 0) - | ((idata->flags & SHMOB_DRM_IFACE_FL_DIPOL) ? LDMT1R_DIPOL : 0) - | ((idata->flags & SHMOB_DRM_IFACE_FL_DAPOL) ? LDMT1R_DAPOL : 0) - | ((idata->flags & SHMOB_DRM_IFACE_FL_HSCNT) ? LDMT1R_HSCNT : 0) - | ((idata->flags & SHMOB_DRM_IFACE_FL_DWCNT) ? LDMT1R_DWCNT : 0); + | ((dpy_flags & DISPLAY_FLAGS_PIXDATA_POSEDGE) ? LDMT1R_DWPOL : 0) + | ((dpy_flags & DISPLAY_FLAGS_DE_LOW) ? LDMT1R_DIPOL : 0); + lcdc_write(sdev, LDMT1R, value); value = ((mode->hdisplay / 8) << 16)/* HDCN */ @@ -548,7 +548,7 @@ static inline struct shmob_drm_connector *to_shmob_connector(struct drm_connecto static int shmob_drm_connector_get_modes(struct drm_connector *connector) { - struct shmob_drm_device *sdev = to_shmob_device(connector->dev); + struct shmob_drm_connector *scon = to_shmob_connector(connector); struct drm_display_mode *mode; mode = drm_mode_create(connector->dev); @@ -556,18 +556,9 @@ static int shmob_drm_connector_get_modes(struct drm_connector *connector) return 0; mode->type = DRM_MODE_TYPE_PREFERRED | DRM_MODE_TYPE_DRIVER; - mode->clock = sdev->pdata->panel.mode.clock; - mode->hdisplay = sdev->pdata->panel.mode.hdisplay; - mode->hsync_start = sdev->pdata->panel.mode.hsync_start; - mode->hsync_end = sdev->pdata->panel.mode.hsync_end; - mode->htotal = sdev->pdata->panel.mode.htotal; - mode->vdisplay = sdev->pdata->panel.mode.vdisplay; - mode->vsync_start = sdev->pdata->panel.mode.vsync_start; - mode->vsync_end = sdev->pdata->panel.mode.vsync_end; - mode->vtotal = sdev->pdata->panel.mode.vtotal; - mode->flags = sdev->pdata->panel.mode.flags; - - drm_mode_set_name(mode); + + drm_display_mode_from_videomode(scon->mode, mode); + drm_mode_probed_add(connector, mode); return 1; @@ -601,10 +592,12 @@ static const struct drm_connector_funcs connector_funcs = { int shmob_drm_connector_create(struct shmob_drm_device *sdev, struct drm_encoder *encoder) { - struct drm_connector *connector = >connector.connector; + struct shmob_drm_connector *scon = >connector; + struct drm_connector *connector = >connector; int ret; - sdev->connector.encoder = encoder; + scon->encoder = encoder; + scon->mode =
[PATCH v3 07/41] drm: renesas: shmobile: Add support for Runtime PM
The SH-Mobile LCD Controller is part of a PM Domain on all relevant SoCs (clock domain on all, power domain on some). Hence it may not be sufficient to manage the LCDC module clock explicitly (e.g. if the selected clock source differs from SHMOB_DRM_CLK_BUS). Fix this by using Runtime PM for all clock handling. Add an explicit dependency on CONFIG_PM, which should already be met on all affected platforms. Signed-off-by: Geert Uytterhoeven --- v3: - No changes, v2: - Move explicit clock handling to Runtime PM callbacks, - Move devm_pm_runtime_enable() after shmob_drm_setup_clocks(), - Depend on PM. --- drivers/gpu/drm/renesas/shmobile/Kconfig | 2 +- .../gpu/drm/renesas/shmobile/shmob_drm_crtc.c | 32 ++-- .../gpu/drm/renesas/shmobile/shmob_drm_drv.c | 38 +-- 3 files changed, 40 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/drm/renesas/shmobile/Kconfig b/drivers/gpu/drm/renesas/shmobile/Kconfig index ad14112999ad8aba..ba941587ca70e08c 100644 --- a/drivers/gpu/drm/renesas/shmobile/Kconfig +++ b/drivers/gpu/drm/renesas/shmobile/Kconfig @@ -1,7 +1,7 @@ # SPDX-License-Identifier: GPL-2.0 config DRM_SHMOBILE tristate "DRM Support for SH Mobile" - depends on DRM + depends on DRM && PM depends on ARCH_RENESAS || ARCH_SHMOBILE || COMPILE_TEST select BACKLIGHT_CLASS_DEVICE select DRM_KMS_HELPER diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c index fbfd906844da490c..2d9ae0c6ab7b18a8 100644 --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c @@ -9,6 +9,7 @@ #include #include +#include #include #include @@ -34,29 +35,6 @@ * TODO: panel support */ -/* - - * Clock management - */ - -static int shmob_drm_clk_on(struct shmob_drm_device *sdev) -{ - int ret; - - if (sdev->clock) { - ret = clk_prepare_enable(sdev->clock); - if (ret < 0) - return ret; - } - - return 0; -} - -static void shmob_drm_clk_off(struct shmob_drm_device *sdev) -{ - if (sdev->clock) - clk_disable_unprepare(sdev->clock); -} - /* - * CRTC */ @@ -170,9 +148,8 @@ static void shmob_drm_crtc_start(struct shmob_drm_crtc *scrtc) if (WARN_ON(format == NULL)) return; - /* Enable clocks before accessing the hardware. */ - ret = shmob_drm_clk_on(sdev); - if (ret < 0) + ret = pm_runtime_resume_and_get(sdev->dev); + if (ret) return; /* Reset and enable the LCDC. */ @@ -268,8 +245,7 @@ static void shmob_drm_crtc_stop(struct shmob_drm_crtc *scrtc) /* Disable the display output. */ lcdc_write(sdev, LDCNT1R, 0); - /* Stop clocks. */ - shmob_drm_clk_off(sdev); + pm_runtime_put(sdev->dev); scrtc->started = false; } diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c index 30493ce874192e3e..3fc7d820abdc61d4 100644 --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -165,8 +166,35 @@ static int shmob_drm_pm_resume(struct device *dev) return 0; } -static DEFINE_SIMPLE_DEV_PM_OPS(shmob_drm_pm_ops, - shmob_drm_pm_suspend, shmob_drm_pm_resume); +static int shmob_drm_pm_runtime_suspend(struct device *dev) +{ + struct shmob_drm_device *sdev = dev_get_drvdata(dev); + + if (sdev->clock) + clk_disable_unprepare(sdev->clock); + + return 0; +} + +static int shmob_drm_pm_runtime_resume(struct device *dev) +{ + struct shmob_drm_device *sdev = dev_get_drvdata(dev); + int ret; + + if (sdev->clock) { + ret = clk_prepare_enable(sdev->clock); + if (ret < 0) + return ret; + } + + return 0; +} + +static const struct dev_pm_ops shmob_drm_pm_ops = { + SYSTEM_SLEEP_PM_OPS(shmob_drm_pm_suspend, shmob_drm_pm_resume) + RUNTIME_PM_OPS(shmob_drm_pm_runtime_suspend, + shmob_drm_pm_runtime_resume, NULL) +}; /* - * Platform driver @@ -220,6 +248,10 @@ static int shmob_drm_probe(struct platform_device *pdev) if (ret < 0) return ret; + ret = devm_pm_runtime_enable(>dev); + if (ret) + return ret; + ret = shmob_drm_init_interface(sdev); if (ret < 0) return ret; @@ -291,7 +323,7 @@ static struct platform_driver
[PATCH v3 14/41] drm: renesas: shmobile: Rename input clocks
From: Laurent Pinchart Prepare for DT bindings by using more appropriate names for the input clocks. Note that all LDDCKR_ICKSEL_* definitions but the one for the bus clock are valid only for SH7724, so the clock selection code needs to be updated when extending clock support to other SoCs. Signed-off-by: Laurent Pinchart [geert: Add note] Signed-off-by: Geert Uytterhoeven Reviewed-by: Sui Jingfeng --- v3: - No changes, v2: - Add Reviewed-by. --- drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c index 64fc3fb02e6c6dc8..1157b4894ff319cd 100644 --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c @@ -74,15 +74,15 @@ static int shmob_drm_setup_clocks(struct shmob_drm_device *sdev, switch (clksrc) { case SHMOB_DRM_CLK_BUS: - clkname = "bus_clk"; + clkname = "fck"; sdev->lddckr = LDDCKR_ICKSEL_BUS; break; case SHMOB_DRM_CLK_PERIPHERAL: - clkname = "peripheral_clk"; + clkname = "media"; sdev->lddckr = LDDCKR_ICKSEL_MIPI; break; case SHMOB_DRM_CLK_EXTERNAL: - clkname = NULL; + clkname = "lclk"; sdev->lddckr = LDDCKR_ICKSEL_HDMI; break; default: -- 2.34.1
[PATCH v3 29/41] drm: renesas: shmobile: Rename shmob_drm_plane.plane
Rename the "plane" member of the shmob_drm_plane subclass structure to "base", to improve readability. Signed-off-by: Geert Uytterhoeven Reviewed-by: Laurent Pinchart Reviewed-by: Sui Jingfeng --- v3: - No changes, v2: - Add Reviewed-by. --- drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c index 3518f8900c0d1f9e..d0a9299784d4a7cc 100644 --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c @@ -20,7 +20,7 @@ #include "shmob_drm_regs.h" struct shmob_drm_plane { - struct drm_plane plane; + struct drm_plane base; unsigned int index; unsigned int alpha; @@ -37,7 +37,7 @@ struct shmob_drm_plane { static inline struct shmob_drm_plane *to_shmob_plane(struct drm_plane *plane) { - return container_of(plane, struct shmob_drm_plane, plane); + return container_of(plane, struct shmob_drm_plane, base); } static void shmob_drm_plane_compute_base(struct shmob_drm_plane *splane, @@ -64,7 +64,7 @@ static void shmob_drm_plane_compute_base(struct shmob_drm_plane *splane, static void __shmob_drm_plane_setup(struct shmob_drm_plane *splane, struct drm_framebuffer *fb) { - struct shmob_drm_device *sdev = to_shmob_device(splane->plane.dev); + struct shmob_drm_device *sdev = to_shmob_device(splane->base.dev); u32 format; /* TODO: Support ROP3 mode */ @@ -216,7 +216,7 @@ struct drm_plane *shmob_drm_plane_create(struct shmob_drm_device *sdev, funcs = _drm_overlay_plane_funcs; splane = drmm_universal_plane_alloc(>ddev, - struct shmob_drm_plane, plane, 1, + struct shmob_drm_plane, base, 1, funcs, formats, ARRAY_SIZE(formats), NULL, type, NULL); @@ -226,5 +226,5 @@ struct drm_plane *shmob_drm_plane_create(struct shmob_drm_device *sdev, splane->index = index; splane->alpha = 255; - return >plane; + return >base; } -- 2.34.1
[PATCH v3 16/41] drm: renesas: shmobile: Improve error handling
Prepare for DT conversion, where panel probe can be deferred, by streamlining error propagation and handling: - Use dev_err_probe() to avoid printing error messages in case of probe deferral, - Propagate errors where needed. Signed-off-by: Geert Uytterhoeven Reviewed-by: Laurent Pinchart --- v3: - No changes, v2: - Add Reviewed-by. --- drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c | 3 ++- drivers/gpu/drm/renesas/shmobile/shmob_drm_kms.c | 14 +++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c index 782767fc66d00c4f..91daab80b0ede058 100644 --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_drv.c @@ -255,7 +255,8 @@ static int shmob_drm_probe(struct platform_device *pdev) ret = shmob_drm_modeset_init(sdev); if (ret < 0) { - dev_err(>dev, "failed to initialize mode setting\n"); + dev_err_probe(>dev, ret, + "failed to initialize mode setting\n"); goto err_free_drm_dev; } diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_kms.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_kms.c index 3051318ddc7999bc..1a62e7f8a8a9e6df 100644 --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_kms.c +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_kms.c @@ -157,9 +157,17 @@ int shmob_drm_modeset_init(struct shmob_drm_device *sdev) if (ret) return ret; - shmob_drm_crtc_create(sdev); - shmob_drm_encoder_create(sdev); - shmob_drm_connector_create(sdev, >encoder); + ret = shmob_drm_crtc_create(sdev); + if (ret < 0) + return ret; + + ret = shmob_drm_encoder_create(sdev); + if (ret < 0) + return ret; + + ret = shmob_drm_connector_create(sdev, >encoder); + if (ret < 0) + return ret; drm_kms_helper_poll_init(sdev->ddev); -- 2.34.1
[PATCH v3 21/41] drm: renesas: shmobile: Convert container helpers to static inline functions
Replace to conversion helper macros using container_of() by static inline functions, to improve type-safety. Signed-off-by: Geert Uytterhoeven Reviewed-by: Laurent Pinchart --- v3: - No changes, v2: - Add Reviewed-by. --- drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c | 11 --- drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c | 5 - 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c index 9c66e00ed70ea582..207fa98fe76d6f88 100644 --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c @@ -258,7 +258,10 @@ static void shmob_drm_crtc_update_base(struct shmob_drm_crtc *scrtc) lcdc_write(sdev, LDRCNTR, lcdc_read(sdev, LDRCNTR) ^ LDRCNTR_MRS); } -#define to_shmob_crtc(c) container_of(c, struct shmob_drm_crtc, crtc) +static inline struct shmob_drm_crtc *to_shmob_crtc(struct drm_crtc *crtc) +{ + return container_of(crtc, struct shmob_drm_crtc, crtc); +} static void shmob_drm_crtc_dpms(struct drm_crtc *crtc, int mode) { @@ -538,8 +541,10 @@ int shmob_drm_encoder_create(struct shmob_drm_device *sdev) * Connector */ -#define to_shmob_connector(c) \ - container_of(c, struct shmob_drm_connector, connector) +static inline struct shmob_drm_connector *to_shmob_connector(struct drm_connector *connector) +{ + return container_of(connector, struct shmob_drm_connector, connector); +} static int shmob_drm_connector_get_modes(struct drm_connector *connector) { diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c index 17e66a018689f648..258288c80756bf16 100644 --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c @@ -34,7 +34,10 @@ struct shmob_drm_plane { unsigned int crtc_h; }; -#define to_shmob_plane(p) container_of(p, struct shmob_drm_plane, plane) +static inline struct shmob_drm_plane *to_shmob_plane(struct drm_plane *plane) +{ + return container_of(plane, struct shmob_drm_plane, plane); +} static void shmob_drm_plane_compute_base(struct shmob_drm_plane *splane, struct drm_framebuffer *fb, -- 2.34.1