Re: [linux-next:master] [mm/slab] 7bd230a266: WARNING:at_mm/util.c:#kvmalloc_node_noprof
On Sun, May 19, 2024 at 07:06:45PM -0400, Kent Overstreet wrote:
> this looks like an i915 bug
Yeah, agreed.
> On Wed, May 15, 2024 at 10:41:19AM +0800, kernel test robot wrote:
[...]
> > [test failed on linux-next/master 6ba6c795dc73c22ce2c86006f17c4aa802db2a60]
[...]
> >
> > 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-lkp/202405151008.6ddd1aaf-oliver.s...@intel.com
> >
> >
> > [ 940.101700][ T5353] [ cut here ]
> > [ 940.107107][ T5353] WARNING: CPU: 1 PID: 5353 at mm/util.c:649
> > kvmalloc_node_noprof (mm/util.c:649 (discriminator 1))
This is:
/* Don't even allow crazy sizes */
if (unlikely(size > INT_MAX)) {
WARN_ON_ONCE(!(flags & __GFP_NOWARN));
> > [ 940.307791][ T5353] Call Trace:
[...]
> > [ 940.351795][ T5353] eb_copy_relocations
> > (drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1685) i915
And this is:
const unsigned int nreloc = eb->exec[i].relocation_count;
...
size = nreloc * sizeof(*relocs);
relocs = kvmalloc_array(1, size, GFP_KERNEL);
So something isn't checking the "relocation_count" size that I assume is
coming in from the ioctl?
-Kees
--
Kees Cook
Re: [PATCH] drm/bridge: adv7511: Exit interrupt handling when necessary
On 5/20/24 06:11, Dmitry Baryshkov wrote:
> On Thu, May 16, 2024 at 06:10:06PM +0800, Liu Ying wrote:
>> Commit f3d9683346d6 ("drm/bridge: adv7511: Allow IRQ to share GPIO pins")
>> fails to consider the case where adv7511->i2c_main->irq is zero, i.e.,
>> no interrupt requested at all.
>>
>> Without interrupt, adv7511_wait_for_edid() could return -EIO sometimes,
>> because it polls adv7511->edid_read flag by calling adv7511_irq_process()
>> a few times, but adv7511_irq_process() happens to refuse to handle
>> interrupt by returning -ENODATA. Hence, EDID retrieval fails randomly.
>>
>> Fix the issue by checking adv7511->i2c_main->irq before exiting interrupt
>> handling from adv7511_irq_process().
>>
>> Fixes: f3d9683346d6 ("drm/bridge: adv7511: Allow IRQ to share GPIO pins")
>> Signed-off-by: Liu Ying
>> ---
>> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> index 6089b0bb9321..2074fa3c1b7b 100644
>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> @@ -479,7 +479,8 @@ static int adv7511_irq_process(struct adv7511 *adv7511,
>> bool process_hpd)
>> return ret;
>>
>> /* If there is no IRQ to handle, exit indicating no IRQ data */
>> -if (!(irq0 & (ADV7511_INT0_HPD | ADV7511_INT0_EDID_READY)) &&
>> +if (adv7511->i2c_main->irq &&
>> +!(irq0 & (ADV7511_INT0_HPD | ADV7511_INT0_EDID_READY)) &&
>> !(irq1 & ADV7511_INT1_DDC_ERROR))
>> return -ENODATA;
>
> I think it might be better to handle -ENODATA in adv7511_wait_for_edid()
> instead. WDYT?
Then, adv7511_cec_irq_process() will have less chance to be called from
adv7511_irq_process() (assuming CONFIG_DRM_I2C_ADV7511_CEC is defined)
if adv7511->i2c_main->irq is zero.
But, anyway, it seems that commit f3d9683346d6 ("drm/bridge: adv7511:
Allow IRQ to share GPIO pins") is even more broken to handle the CEC case,
as adv7511_cec_adap_enable() may enable some interrupts for CEC.
This is a bit complicated. Thoughts?
Regards,
Liu Ying
Re: [PATCH] drm/msm/adreno: Check for zap node availability
On Fri, May 17, 2024 at 12:50:19PM -0700, Rob Clark wrote:
> From: Rob Clark
>
> This should allow disabling the zap node via an overlay, for slbounce.
>
> Suggested-by: Nikita Travkin
> Signed-off-by: Rob Clark
> ---
> drivers/gpu/drm/msm/adreno/adreno_gpu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index d9ea15994ae9..a00241e3373b 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -46,7 +46,7 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const
> char *fwname,
> }
>
> np = of_get_child_by_name(dev->of_node, "zap-shader");
> - if (!np) {
> + if (!np || !of_device_is_available(np)) {
if (!of_device_is_available(np)) {
would cover both cases and be slightly cleaner imho...
But this looks reasonable either way.
Reviewed-by: Bjorn Andersson
Regards,
Bjorn
> zap_available = false;
> return -ENODEV;
> }
> --
> 2.45.1
>
Re: simpledrm, running display servers, and drivers replacing simpledrm while the display server is running
On Friday, May 10, 2024 9:11:13 AM EDT Jonas Ådahl wrote: > On Fri, May 10, 2024 at 02:45:48PM +0200, Thomas Zimmermann wrote: > > Hi > > > > > (This was discussed on #dri-devel, but I'll reiterate here as well). > > > > > > There are two problems at hand; one is the race condition during boot > > > when the login screen (or whatever display server appears first) is > > > launched with simpledrm, only some moments later having the real GPU > > > driver appear. > > > > > > The other is general purpose GPU hotplugging, including the unplugging > > > the GPU decided by the compositor to be the primary one. > > > > The situation of booting with simpledrm (problem 2) is a special case of > > problem 1. From the kernel's perspective, unloading simpledrm is the same as > > what you call general purpose GPU hotplugging. Even through there is not a > > full GPU, but a trivial scanout buffer. In userspace, you see the same > > sequence of events as in the general case. > > Sure, in a way it is, but the consequence and frequency of occurence is > quite different, so I think it makes sense to think of them as different > problems, since they need different solutions. One is about fixing > userspace components support for arbitrary hotplugging, the other for > mitigating the race condition that caused this discussion to begin with. > > > > > > > > > The latter is something that should be handled in userspace, by > > > compositors, etc, I agree. > > > > > > The former, however, is not properly solved by userspace learning how to > > > deal with primary GPU unplugging and switching to using a real GPU > > > driver, as it'd break the booting and login experience. > > > > > > When it works, i.e. the race condition is not hit, is this: > > > > > > * System boots > > > * Plymouth shows a "splash" screen > > > * The login screen display server is launched with the real GPU driver > > > * The login screen interface is smoothly animating using hardware > > > accelerating, presenting "advanced" graphical content depending on > > > hardware capabilities (e.g. high color bit depth, HDR, and so on) > > > > > > If the race condition is hit, with a compositor supporting primary GPU > > > hotplugging, it'll work like this: > > > > > > * System boots > > > * Plymouth shows a "splash" screen > > > * The login screen display server is launched with simpledrm > > > * Due to using simpldrm, the login screen interface is not animated and > > > just plops up, and no "advanced" graphical content is enabled due to > > > apparent missing hardware capabilities > > > * The real GPU driver appears, the login screen now starts to become > > > animated, and may suddenly change appearance due to capabilties > > > having changed > > > > > > Thus, by just supporting hotplugging the primary GPU in userspace, we'll > > > still end up with a glitchy boot experience, and it forces userspace to > > > add things like sleep(10) to work around this. > > > > > > In other words, fixing userspace is *not* a correct solution to the > > > problem, it's a work around (albeit a behaivor we want for other > > > reasons) for the race condition. > > > > To really fix the flickering, you need to read the old DRM device's atomic > > state and apply it to the new device. Then tell the desktop and applications > > to re-init their rendering stack. > > > > Depending on the DRM driver and its hardware, it might be possible to do > > this without flickering. The key is to not loose the original scanout > > buffer, while not probing the new device driver. But that needs work in each > > individual DRM driver. > > This doesn't sound like it'll fix any flickering as I describe them. > First, the loss of initial animation when the login interface appears is > not something one can "fix", since it has already happened. > I feel like whatever animations that a login screen has though is going to be in the realm of a fade-in animation, or maybe a sliding animation though, or one of those that are more on the simple side. llvmpipe should be good enough for animations like that these days I would think, right? Or is it really bad on very very old CPUs, like say a Pentium III? > Avoiding flickering when switching to the new driver is only possible > if one limits oneself to what simpledrm was capable of doing, i.e. no > HDR signaling etc. > > > > > > > > > Arguably, the only place a more educated guess about whether to wait or > > > not, and if so how long, is the kernel. > > > > As I said before, driver modules come and go and hardware devices come and > > go. > > > > To detect if there might be a native driver waiting to be loaded, you can > > test for > > > > - 'nomodeset' on the command line -> no native driver > > Makes sense to not wait here, and just assume simpledrm forever. > > > - 'systemd-load-modules' not started -> maybe wait > > - look for drivers under /lib/modules//kernel/drivers/gpu/drm/ -> > >
Re: [linux-next:master] [mm/slab] 7bd230a266: WARNING:at_mm/util.c:#kvmalloc_node_noprof
this looks like an i915 bug
On Wed, May 15, 2024 at 10:41:19AM +0800, kernel test robot wrote:
>
>
> Hello,
>
> as we understand, this commit is not the root-cause of this WARNING. the
> WARNING
> just shows in another way by commit changes.
>
> 53ed0af496422959 7bd230a26648ac68ab3731ebbc4
> ---
>fail:runs %reproductionfail:runs
>| | |
> 6:6 -83%:6 dmesg.RIP:kvmalloc_node
>:6 33% 6:6 dmesg.RIP:kvmalloc_node_noprof
> 6:6 -83%:6
> dmesg.WARNING:at_mm/util.c:#kvmalloc_node
>:6 33% 6:6
> dmesg.WARNING:at_mm/util.c:#kvmalloc_node_noprof
>
>
> but we failed to bisect "dmesg.WARNING:at_mm/util.c:#kvmalloc_node".
>
> we still made this report FYI what we observed in our tests, not sure if it
> could give somebody some hints to find the real problem then judge if a fix
> is needed.
>
> below is full report.
>
>
>
> kernel test robot noticed "WARNING:at_mm/util.c:#kvmalloc_node_noprof" on:
>
> commit: 7bd230a26648ac68ab3731ebbc449090f0ac6a37 ("mm/slab: enable slab
> allocation tagging for kmalloc and friends")
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master
>
> [test failed on linux-next/master 6ba6c795dc73c22ce2c86006f17c4aa802db2a60]
>
> in testcase: igt
> version: igt-x86_64-86712f2ef-1_20240511
> with following parameters:
>
> group: gem_exec_reloc
>
>
>
> compiler: gcc-13
> test machine: 20 threads 1 sockets (Commet Lake) with 16G memory
>
> (please refer to attached dmesg/kmsg for entire log/backtrace)
>
>
>
> 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-lkp/202405151008.6ddd1aaf-oliver.s...@intel.com
>
>
> [ 940.101700][ T5353] [ cut here ]
> [ 940.107107][ T5353] WARNING: CPU: 1 PID: 5353 at mm/util.c:649
> kvmalloc_node_noprof (mm/util.c:649 (discriminator 1))
> [ 940.116178][ T5353] Modules linked in: netconsole btrfs blake2b_generic
> xor zstd_compress intel_rapl_msr intel_rapl_common intel_uncore_frequency
> intel_uncore_frequency_common raid6_pq libcrc32c x86_pkg_temp_thermal
> intel_powerclamp coretemp kvm_intel sd_mod t10_pi crc64_rocksoft_generic
> crc64_rocksoft ipmi_devintf crc64 sg ipmi_msghandler kvm crct10dif_pclmul
> crc32_pclmul crc32c_intel ghash_clmulni_intel i915 sha512_ssse3 sdhci_pci
> drm_buddy cqhci ahci rapl intel_gtt drm_display_helper sdhci libahci mei_me
> ttm intel_cstate i2c_designware_platform ppdev intel_uncore
> intel_wmi_thunderbolt wmi_bmof libata mei i2c_designware_core idma64
> drm_kms_helper mmc_core i2c_i801 i2c_smbus intel_pch_thermal parport_pc video
> parport pinctrl_cannonlake wmi acpi_pad acpi_tad serio_raw binfmt_misc drm
> fuse loop dm_mod ip_tables
> [ 940.188041][ T5353] CPU: 1 PID: 5353 Comm: gem_exec_reloc Not tainted
> 6.9.0-rc4-00085-g7bd230a26648 #1
> [ 940.197459][ T5353] RIP: 0010:kvmalloc_node_noprof (mm/util.c:649
> (discriminator 1))
> [ 940.203412][ T5353] Code: 04 a3 0d 00 48 83 c4 18 48 83 c4 08 5b 5d 41 5c
> 41 5d 41 5e c3 cc cc cc cc 49 be 00 00 00 00 00 20 00 00 eb 9f 80 e7 20 75 de
> <0f> 0b eb da 48 c7 c7 10 ec af 84 e8 0e a6 18 00 e9 3f ff ff ff 48
> All code
>
>0: 04 a3 add$0xa3,%al
>2: 0d 00 48 83 c4 or $0xc4834800,%eax
>7: 18 48 83sbb%cl,-0x7d(%rax)
>a: c4 (bad)
>b: 08 5b 5dor %bl,0x5d(%rbx)
>e: 41 5c pop%r12
> 10: 41 5d pop%r13
> 12: 41 5e pop%r14
> 14: c3 retq
> 15: cc int3
> 16: cc int3
> 17: cc int3
> 18: cc int3
> 19: 49 be 00 00 00 00 00movabs $0x2000,%r14
> 20: 20 00 00
> 23: eb 9f jmp0xffc4
> 25: 80 e7 20and$0x20,%bh
> 28: 75 de jne0x8
> 2a:*0f 0b ud2 <-- trapping instruction
> 2c: eb da jmp0x8
> 2e: 48 c7 c7 10 ec af 84mov$0x84afec10,%rdi
> 35: e8 0e a6 18 00 callq 0x18a648
> 3a: e9 3f ff ff ff jmpq 0xff7e
> 3f: 48 rex.W
>
> Code starting with the faulting instruction
> ===
>0: 0f 0b ud2
>2: eb da jmp0xffde
>4: 48 c7 c7 10 ec af 84mov$0x84afec10,%rdi
>b: e8 0e a6 18 00 callq 0x18a61e
> 10: e9 3f ff ff ff jmpq
Re: [PATCH 1/6] drm/bridge: analogix: remove unused struct 'bridge_init'
* Dmitry Baryshkov (dmitry.barysh...@linaro.org) wrote:
> On Sat, May 18, 2024 at 12:24:27AM +0100, li...@treblig.org wrote:
> > From: "Dr. David Alan Gilbert"
> >
> > 'bridge_init' is unused, I think following:
> > commit 6a1688ae8794 ("drm/bridge: ptn3460: Convert to I2C driver model")
> > (which is where a git --follow finds it)
> > Remove it.
>
> Please rephrase the commit message following guidelines in
> Documentation/process. Use Fixes tags if suitable.
I specifically don't want to use Fixes in these set because
there's no need for someone to backport this to older
kernels that use the original, and many backporters
use 'Fixes' as an automated means to find stuff they should
backport.
Other than that, is there something specific you think I've
missed?
(I'm also purposely being less certain here, because --follow
is showing it in a ptn3460 and I don't quite follow
why that changes it here).
Dave
>
> >
> > Build tested.
> >
> > Signed-off-by: Dr. David Alan Gilbert
> > ---
> > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 5 -
> > 1 file changed, 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > index df9370e0ff23..1e03f3525a92 100644
> > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > @@ -36,11 +36,6 @@
> >
> > static const bool verify_fast_training;
> >
> > -struct bridge_init {
> > - struct i2c_client *client;
> > - struct device_node *node;
> > -};
> > -
> > static int analogix_dp_init_dp(struct analogix_dp_device *dp)
> > {
> > int ret;
> > --
> > 2.45.1
> >
>
> --
> With best wishes
> Dmitry
--
-Open up your eyes, open up your mind, open up your code ---
/ Dr. David Alan Gilbert| Running GNU/Linux | Happy \
\dave @ treblig.org | | In Hex /
\ _|_ http://www.treblig.org |___/
Re: [PATCH 1/6] drm/bridge: analogix: remove unused struct 'bridge_init'
On Sat, May 18, 2024 at 12:24:27AM +0100, li...@treblig.org wrote:
> From: "Dr. David Alan Gilbert"
>
> 'bridge_init' is unused, I think following:
> commit 6a1688ae8794 ("drm/bridge: ptn3460: Convert to I2C driver model")
> (which is where a git --follow finds it)
> Remove it.
Please rephrase the commit message following guidelines in
Documentation/process. Use Fixes tags if suitable.
>
> Build tested.
>
> Signed-off-by: Dr. David Alan Gilbert
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 5 -
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index df9370e0ff23..1e03f3525a92 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -36,11 +36,6 @@
>
> static const bool verify_fast_training;
>
> -struct bridge_init {
> - struct i2c_client *client;
> - struct device_node *node;
> -};
> -
> static int analogix_dp_init_dp(struct analogix_dp_device *dp)
> {
> int ret;
> --
> 2.45.1
>
--
With best wishes
Dmitry
Re: [PATCH 8/8] drm/panel: himax-hx83102: use wrapped MIPI DCS functions
On Fri, May 17, 2024 at 02:36:43PM -0700, Douglas Anderson wrote:
> Take advantage of some of the new wrapped routines introduced by
> commit f79d6d28d8fe ("drm/mipi-dsi: wrap more functions for streamline
> handling") to simplify the himax-hx83102 driver a bit more. This gets
> rid of some extra error prints (since the _multi functions all print
> errors for you) and simplifies the code a bit.
>
> One thing here that isn't just refactoring is that in a few places we
> now check with errors with "if (err)" instead of "if (err < 0)". All
> errors are expected to be negative so this is not expected to have any
> impact. The _multi code internally considers anything non-zero to be
> an error so this just makes things consistent.
>
> It can also be noted that hx83102_prepare() has a mix of things that
> can take advantage of _multi calls and things that can't. The cleanest
> seemed to be to use the multi_ctx still but consistently use the
> "accum_err" variable for error returns, though that's definitely a
> style decision with pros and cons.
>
> Signed-off-by: Douglas Anderson
> ---
>
> drivers/gpu/drm/panel/panel-himax-hx83102.c | 92 +++--
> 1 file changed, 28 insertions(+), 64 deletions(-)
>
Reviewed-by: Dmitry Baryshkov
--
With best wishes
Dmitry
Re: [PATCH 7/8] drm/panel: himax-hx83102: Check for errors on the NOP in prepare()
On Fri, May 17, 2024 at 02:36:42PM -0700, Douglas Anderson wrote:
> The mipi_dsi_dcs_nop() function returns an error but we weren't
> checking it in hx83102_prepare(). Add a check. This is highly unlikely
> to matter in practice. If the NOP failed then likely later MIPI
> commands would fail too.
>
> Found by code inspection.
>
> Fixes: 0ef94554dc40 ("drm/panel: himax-hx83102: Break out as separate driver")
> Signed-off-by: Douglas Anderson
> ---
>
> drivers/gpu/drm/panel/panel-himax-hx83102.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
Reviewed-by: Dmitry Baryshkov
--
With best wishes
Dmitry
Re: [PATCH 6/8] drm/panel: himax-hx83102: If prepare fails, disable GPIO before regulators
On Fri, May 17, 2024 at 02:36:41PM -0700, Douglas Anderson wrote:
> The enable GPIO should clearly be set low before turning off
> regulators. That matches both the inverse order that things were
> enabled and also the order in unprepare().
>
> Fixes: 0ef94554dc40 ("drm/panel: himax-hx83102: Break out as separate driver")
> Signed-off-by: Douglas Anderson
> ---
>
> drivers/gpu/drm/panel/panel-himax-hx83102.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Reviewed-by: Dmitry Baryshkov
--
With best wishes
Dmitry
Re: [PATCH 5/8] drm/panel: ilitek-ili9882t: Check for errors on the NOP in prepare()
On Fri, May 17, 2024 at 02:36:40PM -0700, Douglas Anderson wrote:
> The mipi_dsi_dcs_nop() function returns an error but we weren't
> checking it in ili9882t_prepare(). Add a check. This is highly
> unlikely to matter in practice. If the NOP failed then likely later
> MIPI commands would fail too.
>
> Found by code inspection.
>
> Fixes: e2450d32e5fb ("drm/panel: ili9882t: Break out as separate driver")
> Signed-off-by: Douglas Anderson
> ---
>
Reviewed-by: Dmitry Baryshkov
--
With best wishes
Dmitry
Re: [PATCH 4/8] drm/panel: ilitek-ili9882t: If prepare fails, disable GPIO before regulators
On Fri, May 17, 2024 at 02:36:39PM -0700, Douglas Anderson wrote:
> The enable GPIO should clearly be set low before turning off
> regulators. That matches both the inverse order that things were
> enabled and also the order in unprepare().
>
> Fixes: e2450d32e5fb ("drm/panel: ili9882t: Break out as separate driver")
> Signed-off-by: Douglas Anderson
> ---
>
> drivers/gpu/drm/panel/panel-ilitek-ili9882t.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Reviewed-by: Dmitry Baryshkov
--
With best wishes
Dmitry
Re: [PATCH 3/8] drm/panel: boe-tv101wum-nl6: Check for errors on the NOP in prepare()
On Fri, May 17, 2024 at 02:36:38PM -0700, Douglas Anderson wrote:
> The mipi_dsi_dcs_nop() function returns an error but we weren't
> checking it in boe_panel_prepare(). Add a check. This is highly
> unlikely to matter in practice. If the NOP failed then likely later
> MIPI commands would fail too.
>
> Found by code inspection.
>
> Fixes: 812562b8d881 ("drm/panel: boe-tv101wum-nl6: Fine tune the panel power
> sequence")
> Signed-off-by: Douglas Anderson
> ---
>
Reviewed-by: Dmitry Baryshkov
--
With best wishes
Dmitry
Re: [PATCH 2/8] drm/panel: boe-tv101wum-nl6: If prepare fails, disable GPIO before regulators
On Fri, May 17, 2024 at 02:36:37PM -0700, Douglas Anderson wrote:
> The enable GPIO should clearly be set low before turning off
> regulators. That matches both the inverse order that things were
> enabled and also the order in unprepare().
>
> Fixes: a869b9db7adf ("drm/panel: support for boe tv101wum-nl6 wuxga dsi video
> mode panel")
> Signed-off-by: Douglas Anderson
> ---
>
> drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Reviewed-by: Dmitry Baryshkov
--
With best wishes
Dmitry
Re: [PATCH 1/8] drm/panel: himax-hx8394: Handle errors from mipi_dsi_dcs_set_display_on() better
On Fri, May 17, 2024 at 02:36:36PM -0700, Douglas Anderson wrote:
> If mipi_dsi_dcs_set_display_on() returned an error then we'd store
> that in the "ret" variable and jump to error handling. We'd then
> attempt an orderly poweroff. Unfortunately we then blew away the value
> stored in "ret". That means that if the orderly poweroff actually
> worked then we're return 0 (no error) from hx8394_enable() even though
> the panel wasn't enabled.
>
> Fix this by not blowing away "ret".
>
> Found by code inspection.
>
> Fixes: 65dc9360f741 ("drm: panel: Add Himax HX8394 panel controller driver")
> Signed-off-by: Douglas Anderson
> ---
>
> drivers/gpu/drm/panel/panel-himax-hx8394.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
Reviewed-by: Dmitry Baryshkov
--
With best wishes
Dmitry
Re: [PATCH] drm/komeda: remove unused struct 'gamma_curve_segment'
On Thu, May 16, 2024 at 02:37:24PM +0100, li...@treblig.org wrote: > From: "Dr. David Alan Gilbert" > > 'gamma_curve_segment' looks like it has never been used. > Remove it. > > Signed-off-by: Dr. David Alan Gilbert > --- > drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.c | 5 - > 1 file changed, 5 deletions(-) > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH] drm/bridge: adv7511: Exit interrupt handling when necessary
On Thu, May 16, 2024 at 06:10:06PM +0800, Liu Ying wrote:
> Commit f3d9683346d6 ("drm/bridge: adv7511: Allow IRQ to share GPIO pins")
> fails to consider the case where adv7511->i2c_main->irq is zero, i.e.,
> no interrupt requested at all.
>
> Without interrupt, adv7511_wait_for_edid() could return -EIO sometimes,
> because it polls adv7511->edid_read flag by calling adv7511_irq_process()
> a few times, but adv7511_irq_process() happens to refuse to handle
> interrupt by returning -ENODATA. Hence, EDID retrieval fails randomly.
>
> Fix the issue by checking adv7511->i2c_main->irq before exiting interrupt
> handling from adv7511_irq_process().
>
> Fixes: f3d9683346d6 ("drm/bridge: adv7511: Allow IRQ to share GPIO pins")
> Signed-off-by: Liu Ying
> ---
> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index 6089b0bb9321..2074fa3c1b7b 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -479,7 +479,8 @@ static int adv7511_irq_process(struct adv7511 *adv7511,
> bool process_hpd)
> return ret;
>
> /* If there is no IRQ to handle, exit indicating no IRQ data */
> - if (!(irq0 & (ADV7511_INT0_HPD | ADV7511_INT0_EDID_READY)) &&
> + if (adv7511->i2c_main->irq &&
> + !(irq0 & (ADV7511_INT0_HPD | ADV7511_INT0_EDID_READY)) &&
> !(irq1 & ADV7511_INT1_DDC_ERROR))
> return -ENODATA;
I think it might be better to handle -ENODATA in adv7511_wait_for_edid()
instead. WDYT?
--
With best wishes
Dmitry
Re: [PATCH v2 1/3] drm/bridge: tc358767: Use dev_err_probe
On Thu, May 16, 2024 at 08:24:53AM +0200, Alexander Stein wrote: > The function calls preceding these returns can return -EPROBE_DEFER. So > use dev_err_probe to add some information to > /sys/kernel/debug/devices_deferred > > Signed-off-by: Alexander Stein > --- > drivers/gpu/drm/bridge/tc358767.c | 8 +--- > 1 file changed, 5 insertions(+), 3 deletions(-) > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH 0/2] drm/bridge: Add 'struct device *' field to the drm_bridge structure
On Thu, May 16, 2024 at 08:04:59PM +0800, Sui Jingfeng wrote: > > > On 5/16/24 18:40, Sui Jingfeng wrote: > > use 'to_i2c_client(bridge->dev)' to retrieve the pointer > > to_i2c_client(bridge->kdev). > > Besides, this also means that we don't need to add the fwnode > pointer into struct drm_bridge as member. Relief the conflicts > with other reviewers if the work of switching to fwnode is still > needed. As for majorities cases (1 to 1), of_node and fwnode can > be retrieved with 'struct device *' easily. The aux-bridge.c and > aux-hdp-bridge.c can also be converted too easily. > > of_node, fwnode, swnode and device properties are all belong to > the backing device structure itself. It can be more natural to use > device_proterty_read_xxx() APIs after init time, Which in turn > avoid the need to acquire and duplicate all properties another > time in the driver private structure. This doesn't sound 100% correct. This is going to drop the possibile case when bridge driver uses child DT or FW node under the main device node. For example of such usecase see drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c > > We could do the programming around the 'struct device *.', remove > a batch of boilerplate. -- With best wishes Dmitry
Re: [PATCH 11/11] drm/imx/ldb: convert to struct drm_edid
On Tue, May 14, 2024 at 03:55:17PM +0300, Jani Nikula wrote:
> Prefer the struct drm_edid based functions for reading the EDID and
> updating the connector.
>
> Signed-off-by: Jani Nikula
>
> ---
>
> Cc: Philipp Zabel
> Cc: Maarten Lankhorst
> Cc: Maxime Ripard
> Cc: Thomas Zimmermann
> Cc: Shawn Guo
> Cc: Sascha Hauer
> Cc: Pengutronix Kernel Team
> Cc: Fabio Estevam
> Cc: i...@lists.linux.dev
> Cc: linux-arm-ker...@lists.infradead.org
> ---
> drivers/gpu/drm/imx/ipuv3/imx-ldb.c | 24
> 1 file changed, 12 insertions(+), 12 deletions(-)
Note:
https://lore.kernel.org/dri-devel/20240331-drm-imx-cleanup-v2-5-d81c1d1c1...@linaro.org/
Reviewed-by: Dmitry Baryshkov
> diff --git a/drivers/gpu/drm/imx/ipuv3/imx-ldb.c
> b/drivers/gpu/drm/imx/ipuv3/imx-ldb.c
> index 71d70194fcbd..793dfb1a3ed0 100644
> --- a/drivers/gpu/drm/imx/ipuv3/imx-ldb.c
> +++ b/drivers/gpu/drm/imx/ipuv3/imx-ldb.c
> @@ -72,7 +72,7 @@ struct imx_ldb_channel {
> struct device_node *child;
> struct i2c_adapter *ddc;
> int chno;
> - void *edid;
> + const struct drm_edid *drm_edid;
> struct drm_display_mode mode;
> int mode_valid;
> u32 bus_format;
> @@ -142,15 +142,15 @@ static int imx_ldb_connector_get_modes(struct
> drm_connector *connector)
> if (num_modes > 0)
> return num_modes;
>
> - if (!imx_ldb_ch->edid && imx_ldb_ch->ddc)
> - imx_ldb_ch->edid = drm_get_edid(connector, imx_ldb_ch->ddc);
> -
> - if (imx_ldb_ch->edid) {
> - drm_connector_update_edid_property(connector,
> - imx_ldb_ch->edid);
> - num_modes = drm_add_edid_modes(connector, imx_ldb_ch->edid);
> + if (!imx_ldb_ch->drm_edid && imx_ldb_ch->ddc) {
> + imx_ldb_ch->drm_edid = drm_edid_read_ddc(connector,
> + imx_ldb_ch->ddc);
> + drm_edid_connector_update(connector, imx_ldb_ch->drm_edid);
> }
>
> + if (imx_ldb_ch->drm_edid)
> + num_modes = drm_edid_connector_add_modes(connector);
> +
> if (imx_ldb_ch->mode_valid) {
> struct drm_display_mode *mode;
>
> @@ -553,7 +553,6 @@ static int imx_ldb_panel_ddc(struct device *dev,
> struct imx_ldb_channel *channel, struct device_node *child)
> {
> struct device_node *ddc_node;
> - const u8 *edidp;
> int ret;
>
> ddc_node = of_parse_phandle(child, "ddc-i2c-bus", 0);
> @@ -567,6 +566,7 @@ static int imx_ldb_panel_ddc(struct device *dev,
> }
>
> if (!channel->ddc) {
> + const void *edidp;
> int edid_len;
>
> /* if no DDC available, fallback to hardcoded EDID */
> @@ -574,8 +574,8 @@ static int imx_ldb_panel_ddc(struct device *dev,
>
> edidp = of_get_property(child, "edid", _len);
> if (edidp) {
> - channel->edid = kmemdup(edidp, edid_len, GFP_KERNEL);
> - if (!channel->edid)
> + channel->drm_edid = drm_edid_alloc(edidp, edid_len);
> + if (!channel->drm_edid)
> return -ENOMEM;
> } else if (!channel->panel) {
> /* fallback to display-timings node */
> @@ -744,7 +744,7 @@ static void imx_ldb_remove(struct platform_device *pdev)
> for (i = 0; i < 2; i++) {
> struct imx_ldb_channel *channel = _ldb->channel[i];
>
> - kfree(channel->edid);
> + drm_edid_free(channel->drm_edid);
> i2c_put_adapter(channel->ddc);
> }
>
> --
> 2.39.2
>
--
With best wishes
Dmitry
Re: [PATCH 03/11] drm/bridge: analogix_dp: convert to struct drm_edid
On Tue, May 14, 2024 at 03:55:09PM +0300, Jani Nikula wrote: > Prefer the struct drm_edid based functions for reading the EDID and > updating the connector. > > Signed-off-by: Jani Nikula > > --- > > Cc: Andrzej Hajda > Cc: Neil Armstrong > Cc: Robert Foss > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Thomas Zimmermann > --- > .../gpu/drm/bridge/analogix/analogix_dp_core.c| 15 --- > 1 file changed, 8 insertions(+), 7 deletions(-) > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH 10/11] drm/imx/tve: convert to struct drm_edid
On Tue, May 14, 2024 at 03:55:16PM +0300, Jani Nikula wrote:
> Prefer the struct drm_edid based functions for reading the EDID and
> updating the connector.
>
> Signed-off-by: Jani Nikula
>
> ---
>
> Cc: Philipp Zabel
> Cc: Maarten Lankhorst
> Cc: Maxime Ripard
> Cc: Thomas Zimmermann
> Cc: Shawn Guo
> Cc: Sascha Hauer
> Cc: Pengutronix Kernel Team
> Cc: Fabio Estevam
> Cc: i...@lists.linux.dev
> Cc: linux-arm-ker...@lists.infradead.org
> ---
> drivers/gpu/drm/imx/ipuv3/imx-tve.c | 14 ++
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/imx/ipuv3/imx-tve.c
> b/drivers/gpu/drm/imx/ipuv3/imx-tve.c
> index b49bddb85535..29f494bfff67 100644
> --- a/drivers/gpu/drm/imx/ipuv3/imx-tve.c
> +++ b/drivers/gpu/drm/imx/ipuv3/imx-tve.c
> @@ -201,18 +201,16 @@ static int tve_setup_vga(struct imx_tve *tve)
> static int imx_tve_connector_get_modes(struct drm_connector *connector)
> {
> struct imx_tve *tve = con_to_tve(connector);
> - struct edid *edid;
> - int ret = 0;
> + const struct drm_edid *drm_edid;
> + int ret;
>
> if (!tve->ddc)
> return 0;
>
> - edid = drm_get_edid(connector, tve->ddc);
> - if (edid) {
> - drm_connector_update_edid_property(connector, edid);
> - ret = drm_add_edid_modes(connector, edid);
> - kfree(edid);
> - }
> + drm_edid = drm_edid_read_ddc(connector, tve->ddc);
> + drm_edid_connector_update(connector, drm_edid);
> + ret = drm_edid_connector_add_modes(connector);
> + drm_edid_free(drm_edid);
Reviewed-by: Dmitry Baryshkov
Nit: if you change two last lines, you can drop ret= assignment and use
return drm_edid_connector_add_modes(connector).
Maybe we shoud add cocci rule for such cases.
>
> return ret;
> }
> --
> 2.39.2
>
--
With best wishes
Dmitry
Re: [PATCH v2] drm/bridge: tc358767: Enable FRMSYNC timing generator
On Tue, May 14, 2024 at 02:47:24AM +0200, Marek Vasut wrote: > TC9595 datasheet Video Path0 Control (VPCTRL0) Register bit FRMSYNC > description > says "This bit should be disabled only in video mode transmission where Host > transmits video timing together with video data and where pixel clock source > is from DSI clock." . This driver always sources pixel clock from external > xtal, > therefore the FRMSYNC bit must always be enabled, enable it. > > This fixes an actual issue with DSI-to-DPI mode, where the display would > randomly show subtle pixel flickering, or wobble, or shimmering. This is > visible on solid gray color, but the degree of the shimmering differs > between boots, which makes it hard to debug. > > There is a caveat to the FRMSYNC and this bridge pixel PLL, which can only > generate pixel clock with limited accuracy, it may therefore be necessary > to reduce the HFP to fit into line length of input pixel data, to avoid any > possible overflows, which make the output video look striped horizontally. > > Reviewed-by: Robert Foss > Signed-off-by: Marek Vasut > --- > Cc: Adam Ford > Cc: Alexander Stein > Cc: Andrzej Hajda > Cc: Daniel Vetter > Cc: David Airlie > Cc: Frieder Schrempf > Cc: Jernej Skrabec > Cc: Jonas Karlman > Cc: Laurent Pinchart > Cc: Lucas Stach > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Michael Walle > Cc: Neil Armstrong > Cc: Robert Foss > Cc: Thomas Zimmermann > Cc: dri-devel@lists.freedesktop.org > Cc: ker...@dh-electronics.com > --- > V2: - Use plain DIV_ROUND_UP() instead of custom local one > - Add RB from Robert > --- > drivers/gpu/drm/bridge/tc358767.c | 18 +++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH] drm/bridge: adv7511: Attach next bridge without creating connector
On Mon, 13 May 2024 16:02:43 +0800, Liu Ying wrote: > The connector is created by either this ADV7511 bridge driver or > any DRM device driver/previous bridge driver, so this ADV7511 > bridge driver should not let the next bridge driver create connector. > > If the next bridge is a HDMI connector, the next bridge driver > would fail to attach bridge from display_connector_attach() without > the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag. > > [...] Applied to drm-misc-next-fixes, thanks! [1/1] drm/bridge: adv7511: Attach next bridge without creating connector commit: 20da948e3a807c67f0efe4f665e64728be370f3d Best regards, -- With best wishes Dmitry
[git pull] drm urgent (part 2) for 6.10-rc1
Hi Linus, Here is Arunpravin's second fix for the buddy allocator warnings you have been seeing, hopefully this is the end of that, and thanks for your patience. Regards, Dave. drm-next-2024-05-20: drm urgent (the 2nd) for 6.10-rc1 buddy: - fix WARN_ONs during force merge The following changes since commit 431c590c3ab0469dfedad3a832fe73556396ee52: drm/tests: Add a unit test for range bias allocation (2024-05-16 12:50:14 +1000) are available in the Git repository at: https://gitlab.freedesktop.org/drm/kernel.git tags/drm-next-2024-05-20 for you to fetch changes up to 5a5a10d9db77939a22e1d65fc0a4ba6b5d8f4fce: drm/buddy: Fix the warn on's during force merge (2024-05-20 06:42:12 +1000) drm urgent (the 2nd) for 6.10-rc1 buddy: - fix WARN_ONs during force merge Arunpravin Paneer Selvam (1): drm/buddy: Fix the warn on's during force merge drivers/gpu/drm/drm_buddy.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Re: [PATCH v4 2/3] dt-bindings: arm: mediatek: mmsys: Add OF graph support for board path
Hi Angelo,
On 16/05/2024 10:11, AngeloGioacchino Del Regno wrote:
+oneOf:
+ - required:
+ - endpoint@0
+ - required:
+ - endpoint@1
+ - required:
+ - endpoint@2
I'm not sure this is what you expect because I must remove this part to pass
the dt-validate.
I have 2 possible display at the same time (DSI and DPI), then I add this in my
DTSI:
mmsys: syscon@1400 {
compatible = "mediatek,mt8365-mmsys", "syscon";
reg = <0 0x1400 0 0x1000>;
#clock-cells = <1>;
port {
#address-cells = <1>;
#size-cells = <0>;
mmsys_main: endpoint@0 {
reg = <0>;
remote-endpoint = <_in>;
};
mmsys_ext: endpoint@1 {
reg = <1>;
remote-endpoint = <_in>;
};
};
};
But the DTS check returns me an error:
dt-validate -s Documentation/devicetree/bindings
arch/arm64/boot/dts/mediatek/mt8365-evk.dtb
/home/***/linux-upstream/arch/arm64/boot/dts/mediatek/mt8365-evk.dtb: syscon@1400: port:
More than one condition true in oneOf schema:
{'$ref': '/schemas/graph.yaml#/properties/port',
'oneOf': [{'required': ['endpoint@0']},
{'required': ['endpoint@1']},
{'required': ['endpoint@2']}],
'properties': {'endpoint@0': {'$ref': '/schemas/graph.yaml#/properties/endpoint'},
'endpoint@1': {'$ref':
'/schemas/graph.yaml#/properties/endpoint'},
'endpoint@2': {'$ref': '/schemas/graph.yaml#/properties/endpoint'}}}
from schema $id:
http://devicetree.org/schemas/arm/mediatek/mediatek,mmsys.yaml#
In other hand, if I use "ports" to keep only one endpoint for each port:
mmsys: syscon@1400 {
compatible = "mediatek,mt8365-mmsys", "syscon";
reg = <0 0x1400 0 0x1000>;
#clock-cells = <1>;
ports {
#address-cells = <1>;
#size-cells = <0>;
port@0 {
#address-cells = <1>;
#size-cells = <0>;
reg = <0>;
mmsys_main: endpoint@0 {
reg = <0>;
remote-endpoint = <_in>;
};
};
port@1 {
#address-cells = <1>;
#size-cells = <0>;
reg = <1>;
mmsys_ext: endpoint@1 {
reg = <1>;
remote-endpoint = <_in>;
};
};
};
};
The DTS check returns another error:
dt-validate -s Documentation/devicetree/bindings
arch/arm64/boot/dts/mediatek/mt8365-evk.dtb
/home/***/linux-upstream/arch/arm64/boot/dts/mediatek/mt8365-evk.dtb: syscon@1400: 'ports'
does not match any of the regexes: 'pinctrl-[0-9]+'
from schema $id:
http://devicetree.org/schemas/arm/mediatek/mediatek,mmsys.yaml#
Additionally, with the last DTS example, displays aren't working, probably because "ports" isn't
well parsed.
So, I don't know how you want to manage multiple display, but IMHO there are 2
ways:
- removing the current "oneOf".
- adding the "ports" support in the documentation and driver (to be parsed).
Still possible I missed something and I doing shit :)
--
Regards,
Alexandre
[etnaviv-next v14 8/8] drm/etnaviv: Add support for vivante GPU cores attached via PCIe device
Previouly, the component framework is being used to bind multiple platform
GPU devices to a virtual master. The virtual master is manually created by
the driver, and is also a platform device. This is fine and works well for
various SoCs, yet there some hardware venders integrate Vivante GPU cores
into PCIe card and the driver lacks the support for PCIe devices.
Create virtual platform devices as a representation for each GPU IP core,
the manually created platform devices are functional as subcomponent, and
all of them are child of the PCIe master device. The master is real for
PCIe devices, as the PCIe device has already been created by the time the
etnaviv.ko is loaded. Hence, bind all of the virtual child to the real
master, this design reflects the hardware layout perfectly and is
extensible.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/etnaviv/Kconfig | 9 ++
drivers/gpu/drm/etnaviv/Makefile | 2 +
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 12 +-
drivers/gpu/drm/etnaviv/etnaviv_drv.h | 2 +
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 75 --
drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 4 +
drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c | 161 ++
drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h | 44 ++
8 files changed, 293 insertions(+), 16 deletions(-)
create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c
create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h
diff --git a/drivers/gpu/drm/etnaviv/Kconfig b/drivers/gpu/drm/etnaviv/Kconfig
index faa7fc68b009..7cb44f72d512 100644
--- a/drivers/gpu/drm/etnaviv/Kconfig
+++ b/drivers/gpu/drm/etnaviv/Kconfig
@@ -15,6 +15,15 @@ config DRM_ETNAVIV
help
DRM driver for Vivante GPUs.
+config DRM_ETNAVIV_PCI_DRIVER
+ bool "enable ETNAVIV PCI driver support"
+ depends on DRM_ETNAVIV
+ depends on PCI
+ default n
+ help
+ Compile in support for Vivante GPUs attached via PCIe card.
+ Say Y if you have such hardwares.
+
config DRM_ETNAVIV_THERMAL
bool "enable ETNAVIV thermal throttling"
depends on DRM_ETNAVIV
diff --git a/drivers/gpu/drm/etnaviv/Makefile b/drivers/gpu/drm/etnaviv/Makefile
index 46e5ffad69a6..6829e1ebf2db 100644
--- a/drivers/gpu/drm/etnaviv/Makefile
+++ b/drivers/gpu/drm/etnaviv/Makefile
@@ -16,4 +16,6 @@ etnaviv-y := \
etnaviv_perfmon.o \
etnaviv_sched.o
+etnaviv-$(CONFIG_DRM_ETNAVIV_PCI_DRIVER) += etnaviv_pci_drv.o
+
obj-$(CONFIG_DRM_ETNAVIV) += etnaviv.o
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index dc3556aad134..90ee60b00c24 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -24,6 +24,7 @@
#include "etnaviv_gpu.h"
#include "etnaviv_gem.h"
#include "etnaviv_mmu.h"
+#include "etnaviv_pci_drv.h"
#include "etnaviv_perfmon.h"
/*
@@ -568,6 +569,10 @@ static int etnaviv_bind(struct device *dev)
if (ret < 0)
goto out_free_priv;
+ ret = etnaviv_register_irq_handler(dev, priv);
+ if (ret)
+ goto out_unbind;
+
load_gpu(drm);
ret = drm_dev_register(drm, 0);
@@ -596,7 +601,7 @@ static void etnaviv_unbind(struct device *dev)
etnaviv_private_fini(priv);
}
-static const struct component_master_ops etnaviv_master_ops = {
+const struct component_master_ops etnaviv_master_ops = {
.bind = etnaviv_bind,
.unbind = etnaviv_unbind,
};
@@ -740,6 +745,10 @@ static int __init etnaviv_init(void)
if (ret != 0)
goto unregister_gpu_driver;
+ ret = etnaviv_register_pci_driver();
+ if (ret)
+ goto unregister_platform_driver;
+
/*
* If the DT contains at least one available GPU device, instantiate
* the DRM platform device.
@@ -769,6 +778,7 @@ module_init(etnaviv_init);
static void __exit etnaviv_exit(void)
{
etnaviv_destroy_platform_device(_drm);
+ etnaviv_unregister_pci_driver();
platform_driver_unregister(_platform_driver);
platform_driver_unregister(_gpu_driver);
}
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
index 4612843ff9f6..6db26d384cbe 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
@@ -27,6 +27,8 @@ struct etnaviv_gem_object;
struct etnaviv_gem_submit;
struct etnaviv_iommu_global;
+extern const struct component_master_ops etnaviv_master_ops;
+
#define ETNAVIV_SOFTPIN_START_ADDRESS SZ_4M /* must be >= SUBALLOC_SIZE */
struct etnaviv_file_private {
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 3a14e187388a..2b5955693fbb 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -29,6 +30,7 @@
[etnaviv-next v14 7/8] drm/etnaviv: Allow creating subdevices and pass platform specific data
Because some hardware are too complex to be managed by a monolithic driver,
a split of the functionality into child devices can helps to achieve better
modularity.
We will use this function to create subdevice as a repensentation of a
single hardware ip block, so that the same modular approach that works
for ARM-SoC can also works for PCIe cards.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 33 +++
drivers/gpu/drm/etnaviv/etnaviv_drv.h | 9
2 files changed, 37 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 863faac2ea19..dc3556aad134 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -670,16 +670,36 @@ static struct platform_driver etnaviv_platform_driver = {
},
};
-static int etnaviv_create_platform_device(const char *name,
- struct platform_device **ppdev)
+int etnaviv_create_platform_device(struct device *parent,
+ const char *name, int id,
+ struct resource *pres,
+ void *data,
+ struct platform_device **ppdev)
{
struct platform_device *pdev;
int ret;
- pdev = platform_device_alloc(name, PLATFORM_DEVID_NONE);
+ pdev = platform_device_alloc(name, id);
if (!pdev)
return -ENOMEM;
+ pdev->dev.parent = parent;
+
+ if (pres) {
+ ret = platform_device_add_resources(pdev, pres, 1);
+ if (ret) {
+ platform_device_put(pdev);
+ return ret;
+ }
+ }
+
+ if (data) {
+ void *pdata = kmalloc(sizeof(void *), GFP_KERNEL);
+
+ *(void **)pdata = data;
+ pdev->dev.platform_data = pdata;
+ }
+
ret = platform_device_add(pdev);
if (ret) {
platform_device_put(pdev);
@@ -691,7 +711,7 @@ static int etnaviv_create_platform_device(const char *name,
return 0;
}
-static void etnaviv_destroy_platform_device(struct platform_device **ppdev)
+void etnaviv_destroy_platform_device(struct platform_device **ppdev)
{
struct platform_device *pdev = *ppdev;
@@ -728,7 +748,10 @@ static int __init etnaviv_init(void)
if (np) {
of_node_put(np);
- ret = etnaviv_create_platform_device("etnaviv", _drm);
+ ret = etnaviv_create_platform_device(NULL, "etnaviv",
+PLATFORM_DEVID_NONE,
+NULL, NULL,
+_drm);
if (ret)
goto unregister_platform_driver;
}
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
index 4b59fdb457b7..4612843ff9f6 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -98,6 +99,14 @@ bool etnaviv_cmd_validate_one(struct etnaviv_gpu *gpu,
u32 *stream, unsigned int size,
struct drm_etnaviv_gem_submit_reloc *relocs, unsigned int reloc_size);
+int etnaviv_create_platform_device(struct device *parent,
+ const char *name, int id,
+ struct resource *pres,
+ void *data,
+ struct platform_device **ppdev);
+
+void etnaviv_destroy_platform_device(struct platform_device **ppdev);
+
#ifdef CONFIG_DEBUG_FS
void etnaviv_gem_describe_objects(struct etnaviv_drm_private *priv,
struct seq_file *m);
--
2.34.1
[etnaviv-next v14 6/8] drm/etnaviv: Replace the '>dev' with 'dev'
In the etnaviv_pdev_probe(), etnaviv_gpu_platform_probe() function, the
value of '>dev' has been cached to the 'dev' local auto variable.
But part of callers use 'dev' as argument, while the rest use '>dev'.
To keep it consistent, use 'dev' uniformly.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 10 +-
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 16
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 986fd68b489a..863faac2ea19 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -614,7 +614,7 @@ static int etnaviv_pdev_probe(struct platform_device *pdev)
if (!of_device_is_available(core_node))
continue;
- drm_of_component_match_add(>dev, ,
+ drm_of_component_match_add(dev, ,
component_compare_of,
core_node);
}
} else {
@@ -637,9 +637,9 @@ static int etnaviv_pdev_probe(struct platform_device *pdev)
* bit to make sure we are allocating the command buffers and
* TLBs in the lower 4 GiB address space.
*/
- if (dma_set_mask(>dev, DMA_BIT_MASK(40)) ||
- dma_set_coherent_mask(>dev, DMA_BIT_MASK(32))) {
- dev_dbg(>dev, "No suitable DMA available\n");
+ if (dma_set_mask(dev, DMA_BIT_MASK(40)) ||
+ dma_set_coherent_mask(dev, DMA_BIT_MASK(32))) {
+ dev_dbg(dev, "No suitable DMA available\n");
return -ENODEV;
}
@@ -650,7 +650,7 @@ static int etnaviv_pdev_probe(struct platform_device *pdev)
*/
first_node = etnaviv_of_first_available_node();
if (first_node) {
- of_dma_configure(>dev, first_node, true);
+ of_dma_configure(dev, first_node, true);
of_node_put(first_node);
}
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index aa15682f94db..3a14e187388a 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1891,7 +1891,7 @@ static int etnaviv_gpu_platform_probe(struct
platform_device *pdev)
if (!gpu)
return -ENOMEM;
- gpu->dev = >dev;
+ gpu->dev = dev;
mutex_init(>lock);
mutex_init(>sched_lock);
@@ -1905,8 +1905,8 @@ static int etnaviv_gpu_platform_probe(struct
platform_device *pdev)
if (gpu->irq < 0)
return gpu->irq;
- err = devm_request_irq(>dev, gpu->irq, irq_handler, 0,
- dev_name(gpu->dev), gpu);
+ err = devm_request_irq(dev, gpu->irq, irq_handler, 0,
+ dev_name(dev), gpu);
if (err) {
dev_err(dev, "failed to request IRQ%u: %d\n", gpu->irq, err);
return err;
@@ -1925,13 +1925,13 @@ static int etnaviv_gpu_platform_probe(struct
platform_device *pdev)
* autosuspend delay is rather arbitary: no measurements have
* yet been performed to determine an appropriate value.
*/
- pm_runtime_use_autosuspend(gpu->dev);
- pm_runtime_set_autosuspend_delay(gpu->dev, 200);
- pm_runtime_enable(gpu->dev);
+ pm_runtime_use_autosuspend(dev);
+ pm_runtime_set_autosuspend_delay(dev, 200);
+ pm_runtime_enable(dev);
- err = component_add(>dev, _ops);
+ err = component_add(dev, _ops);
if (err < 0) {
- dev_err(>dev, "failed to register component: %d\n", err);
+ dev_err(dev, "failed to register component: %d\n", err);
return err;
}
--
2.34.1
[etnaviv-next v14 5/8] drm/etnaviv: Add support for cached coherent caching mode
Many modern CPUs and/or platforms choose to define their peripheral devices
as cached coherent by default, to be specific, the PCH is capable of
snooping CPU's cache. When hit the peripheral devices will access data
directly from CPU's cache. This means that device drivers do not need to
maintain the coherency issue between a processor and peripheral I/O for
the cached buffers. Hence, it dosen't need us to sync manually on the
software side, which is useful to avoid some overheads, especially for
userspace, but userspace is not known yet.
Probe the hardware maintained cached coherent support of the host platform
with the dev_is_dma_coherent() function, and store the result in struct
etnaviv_drm_private. As this is a platform implementation-defined hardware
feature and again is meant to be shared by all GPU cores. And expose it
via etnaviv parameter mechanism to let userspace know.
Please note that write-combine mapping out of scope of the discussion
and therefore is not being addressed.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 3 +++
drivers/gpu/drm/etnaviv/etnaviv_drv.h | 9 +
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 4
include/uapi/drm/etnaviv_drm.h| 1 +
4 files changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index e3eb31ba9a2b..986fd68b489a 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -5,6 +5,7 @@
#include
#include
+#include
#include
#include
#include
@@ -57,6 +58,8 @@ static int etnaviv_private_init(struct device *dev,
return -ENOMEM;
}
+ priv->cached_coherent = dev_is_dma_coherent(dev);
+
return 0;
}
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
index 1f9b50b5a6aa..4b59fdb457b7 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
@@ -46,6 +46,15 @@ struct etnaviv_drm_private {
struct xarray active_contexts;
u32 next_context_id;
+ /*
+* If true, the cached mapping is consistent for all CPU cores and
+* peripheral bus masters in the system. It means that both of the
+* CPU and GPU will see the same data if the buffer being accessed
+* is cached. And the coherency is guaranteed by the host platform
+* specific hardwares.
+*/
+ bool cached_coherent;
+
/* list of GEM objects: */
struct mutex gem_lock;
struct list_head gem_list;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 02d7efdc82c0..aa15682f94db 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -164,6 +164,10 @@ int etnaviv_gpu_get_param(struct etnaviv_gpu *gpu, u32
param, u64 *value)
*value = gpu->identity.eco_id;
break;
+ case ETNAVIV_PARAM_CACHED_COHERENT:
+ *value = priv->cached_coherent;
+ break;
+
default:
DBG("%s: invalid param: %u", dev_name(gpu->dev), param);
return -EINVAL;
diff --git a/include/uapi/drm/etnaviv_drm.h b/include/uapi/drm/etnaviv_drm.h
index af024d90453d..61eaa8cd0f5e 100644
--- a/include/uapi/drm/etnaviv_drm.h
+++ b/include/uapi/drm/etnaviv_drm.h
@@ -77,6 +77,7 @@ struct drm_etnaviv_timespec {
#define ETNAVIV_PARAM_GPU_PRODUCT_ID0x1c
#define ETNAVIV_PARAM_GPU_CUSTOMER_ID 0x1d
#define ETNAVIV_PARAM_GPU_ECO_ID0x1e
+#define ETNAVIV_PARAM_CACHED_COHERENT 0x1f
#define ETNA_MAX_PIPES 4
--
2.34.1
[etnaviv-next v14 4/8] drm/etnaviv: Fix wrong cache property being used for vmap()
In the etnaviv_gem_vmap_impl() function, the driver vmap whatever buffers
with Write-Combine page property. This is unreasonable, as some platforms
are cached coherent. And cached buffers should be mapped with cached page
property.
Fixes: a0a5ab3e99b8 ("drm/etnaviv: call correct function when trying to vmap a
DMABUF")
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/etnaviv/etnaviv_gem.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index aa95a5e98374..eed98bb9e446 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -342,6 +342,7 @@ void *etnaviv_gem_vmap(struct drm_gem_object *obj)
static void *etnaviv_gem_vmap_impl(struct etnaviv_gem_object *obj)
{
struct page **pages;
+ pgprot_t prot;
lockdep_assert_held(>lock);
@@ -349,8 +350,19 @@ static void *etnaviv_gem_vmap_impl(struct
etnaviv_gem_object *obj)
if (IS_ERR(pages))
return NULL;
- return vmap(pages, obj->base.size >> PAGE_SHIFT,
- VM_MAP, pgprot_writecombine(PAGE_KERNEL));
+ switch (obj->flags) {
+ case ETNA_BO_CACHED:
+ prot = PAGE_KERNEL;
+ break;
+ case ETNA_BO_UNCACHED:
+ prot = pgprot_noncached(PAGE_KERNEL);
+ break;
+ case ETNA_BO_WC:
+ default:
+ prot = pgprot_writecombine(PAGE_KERNEL);
+ }
+
+ return vmap(pages, obj->base.size >> PAGE_SHIFT, VM_MAP, prot);
}
static inline enum dma_data_direction etnaviv_op_to_dma_dir(u32 op)
--
2.34.1
[etnaviv-next v14 3/8] drm/etnaviv: Embed struct drm_device into struct etnaviv_drm_private
Both the instance of struct drm_device and the instance of struct
etnaviv_drm_private are intended to be shared by all GPU cores, both have
only one instance created across drm/etnaviv driver. After embedded in,
the whole structure can be allocated with devm_drm_dev_alloc(). And the
DRM device created is automatically put on driver detach, so we don't need
to call drm_dev_put() explicitly on driver leave. It's also eliminate the
need to use the .dev_private member, which is deprecated according to the
drm document. We can also use container_of() to retrieve pointer for the
containing structure.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c| 65
drivers/gpu/drm/etnaviv/etnaviv_drv.h| 7 +++
drivers/gpu/drm/etnaviv/etnaviv_gem.c| 6 +-
drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 2 +-
drivers/gpu/drm/etnaviv/etnaviv_gpu.c| 6 +-
drivers/gpu/drm/etnaviv/etnaviv_mmu.c| 4 +-
6 files changed, 40 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 22c78bc944c4..e3eb31ba9a2b 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -41,14 +41,9 @@ static struct device_node
*etnaviv_of_first_available_node(void)
return NULL;
}
-static struct etnaviv_drm_private *etnaviv_alloc_private(struct device *dev)
+static int etnaviv_private_init(struct device *dev,
+ struct etnaviv_drm_private *priv)
{
- struct etnaviv_drm_private *priv;
-
- priv = kzalloc(sizeof(*priv), GFP_KERNEL);
- if (!priv)
- return ERR_PTR(-ENOMEM);
-
xa_init_flags(>active_contexts, XA_FLAGS_ALLOC);
mutex_init(>gem_lock);
@@ -58,15 +53,14 @@ static struct etnaviv_drm_private
*etnaviv_alloc_private(struct device *dev)
priv->cmdbuf_suballoc = etnaviv_cmdbuf_suballoc_new(dev);
if (IS_ERR(priv->cmdbuf_suballoc)) {
- kfree(priv);
dev_err(dev, "Failed to create cmdbuf suballocator\n");
- return ERR_PTR(-ENOMEM);
+ return -ENOMEM;
}
- return priv;
+ return 0;
}
-static void etnaviv_free_private(struct etnaviv_drm_private *priv)
+static void etnaviv_private_fini(struct etnaviv_drm_private *priv)
{
if (!priv)
return;
@@ -76,13 +70,11 @@ static void etnaviv_free_private(struct etnaviv_drm_private
*priv)
etnaviv_cmdbuf_suballoc_destroy(priv->cmdbuf_suballoc);
xa_destroy(>active_contexts);
-
- kfree(priv);
}
static void load_gpu(struct drm_device *dev)
{
- struct etnaviv_drm_private *priv = dev->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
unsigned int i;
for (i = 0; i < ETNA_MAX_PIPES; i++) {
@@ -100,7 +92,7 @@ static void load_gpu(struct drm_device *dev)
static int etnaviv_open(struct drm_device *dev, struct drm_file *file)
{
- struct etnaviv_drm_private *priv = dev->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
struct etnaviv_file_private *ctx;
int ret, i;
@@ -143,7 +135,7 @@ static int etnaviv_open(struct drm_device *dev, struct
drm_file *file)
static void etnaviv_postclose(struct drm_device *dev, struct drm_file *file)
{
- struct etnaviv_drm_private *priv = dev->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
struct etnaviv_file_private *ctx = file->driver_priv;
unsigned int i;
@@ -168,7 +160,7 @@ static void etnaviv_postclose(struct drm_device *dev,
struct drm_file *file)
#ifdef CONFIG_DEBUG_FS
static int etnaviv_gem_show(struct drm_device *dev, struct seq_file *m)
{
- struct etnaviv_drm_private *priv = dev->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
etnaviv_gem_describe_objects(priv, m);
@@ -262,7 +254,7 @@ static int show_each_gpu(struct seq_file *m, void *arg)
{
struct drm_info_node *node = (struct drm_info_node *) m->private;
struct drm_device *dev = node->minor->dev;
- struct etnaviv_drm_private *priv = dev->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
struct etnaviv_gpu *gpu;
int (*show)(struct etnaviv_gpu *gpu, struct seq_file *m) =
node->info_ent->data;
@@ -305,7 +297,7 @@ static void etnaviv_debugfs_init(struct drm_minor *minor)
static int etnaviv_ioctl_get_param(struct drm_device *dev, void *data,
struct drm_file *file)
{
- struct etnaviv_drm_private *priv = dev->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
struct drm_etnaviv_param *args = data;
struct etnaviv_gpu *gpu;
@@ -398,7 +390,7 @@ static int etnaviv_ioctl_wait_fence(struct drm_device *dev,
void *data,
struct drm_file
[etnaviv-next v14 2/8] drm/etnaviv: Add constructor and destructor for the etnaviv_drm_private structure
Because there are a lot of data members in the struct etnaviv_drm_private,
which are intended to be shared by all GPU cores. It can be lengthy and
daunting on error handling, the 'gem_lock' of struct etnaviv_drm_private
just be forgeten to destroy on driver leave.
Switch to use the dedicated helpers introduced, etnaviv_bind() and
etnaviv_unbind() gets simplified. Another potential benefit is that
we could put the struct drm_device into struct etnaviv_drm_private
in the future, which made them share the same life time.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 72 +--
1 file changed, 46 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 6500f3999c5f..22c78bc944c4 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -41,6 +41,45 @@ static struct device_node
*etnaviv_of_first_available_node(void)
return NULL;
}
+static struct etnaviv_drm_private *etnaviv_alloc_private(struct device *dev)
+{
+ struct etnaviv_drm_private *priv;
+
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return ERR_PTR(-ENOMEM);
+
+ xa_init_flags(>active_contexts, XA_FLAGS_ALLOC);
+
+ mutex_init(>gem_lock);
+ INIT_LIST_HEAD(>gem_list);
+ priv->num_gpus = 0;
+ priv->shm_gfp_mask = GFP_HIGHUSER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN;
+
+ priv->cmdbuf_suballoc = etnaviv_cmdbuf_suballoc_new(dev);
+ if (IS_ERR(priv->cmdbuf_suballoc)) {
+ kfree(priv);
+ dev_err(dev, "Failed to create cmdbuf suballocator\n");
+ return ERR_PTR(-ENOMEM);
+ }
+
+ return priv;
+}
+
+static void etnaviv_free_private(struct etnaviv_drm_private *priv)
+{
+ if (!priv)
+ return;
+
+ mutex_destroy(>gem_lock);
+
+ etnaviv_cmdbuf_suballoc_destroy(priv->cmdbuf_suballoc);
+
+ xa_destroy(>active_contexts);
+
+ kfree(priv);
+}
+
static void load_gpu(struct drm_device *dev)
{
struct etnaviv_drm_private *priv = dev->dev_private;
@@ -521,35 +560,21 @@ static int etnaviv_bind(struct device *dev)
if (IS_ERR(drm))
return PTR_ERR(drm);
- priv = kzalloc(sizeof(*priv), GFP_KERNEL);
- if (!priv) {
- dev_err(dev, "failed to allocate private data\n");
- ret = -ENOMEM;
+ priv = etnaviv_alloc_private(dev);
+ if (IS_ERR(priv)) {
+ ret = PTR_ERR(priv);
goto out_put;
}
+
drm->dev_private = priv;
dma_set_max_seg_size(dev, SZ_2G);
- xa_init_flags(>active_contexts, XA_FLAGS_ALLOC);
-
- mutex_init(>gem_lock);
- INIT_LIST_HEAD(>gem_list);
- priv->num_gpus = 0;
- priv->shm_gfp_mask = GFP_HIGHUSER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN;
-
- priv->cmdbuf_suballoc = etnaviv_cmdbuf_suballoc_new(drm->dev);
- if (IS_ERR(priv->cmdbuf_suballoc)) {
- dev_err(drm->dev, "Failed to create cmdbuf suballocator\n");
- ret = PTR_ERR(priv->cmdbuf_suballoc);
- goto out_free_priv;
- }
-
dev_set_drvdata(dev, drm);
ret = component_bind_all(dev, drm);
if (ret < 0)
- goto out_destroy_suballoc;
+ goto out_free_priv;
load_gpu(drm);
@@ -561,10 +586,8 @@ static int etnaviv_bind(struct device *dev)
out_unbind:
component_unbind_all(dev, drm);
-out_destroy_suballoc:
- etnaviv_cmdbuf_suballoc_destroy(priv->cmdbuf_suballoc);
out_free_priv:
- kfree(priv);
+ etnaviv_free_private(priv);
out_put:
drm_dev_put(drm);
@@ -580,12 +603,9 @@ static void etnaviv_unbind(struct device *dev)
component_unbind_all(dev, drm);
- etnaviv_cmdbuf_suballoc_destroy(priv->cmdbuf_suballoc);
-
- xa_destroy(>active_contexts);
+ etnaviv_free_private(priv);
drm->dev_private = NULL;
- kfree(priv);
drm_dev_put(drm);
}
--
2.34.1
[etnaviv-next v14 1/8] drm/etnaviv: Add a dedicated helper function to get various clocks
Because the current implementation is DT-based, this only works when the
host platform has the DT support. The problem is that some host platforms
does not provide DT-based clocks drivers, as a result, the driver rage
quit.
PLL hardwares are typically provided by the host platform, which is part
of the entire clock tree. The PLL hardware provide clock pulse to the GPU
core, but it's not belong to the GPU corei itself. PLL registers can be
manipulated directly by the device driver. Hence, it may need dedicated
clock driver.
Add a the etnaviv_gpu_clk_get() function to group similar code blocks,
which make it easier to call this function on the platform where it works.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 53 ---
1 file changed, 32 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index d84d73c197fc..e0c36f564fa6 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1605,6 +1605,35 @@ static irqreturn_t irq_handler(int irq, void *data)
return ret;
}
+static int etnaviv_gpu_clk_get(struct etnaviv_gpu *gpu)
+{
+ struct device *dev = gpu->dev;
+
+ gpu->clk_reg = devm_clk_get_optional(dev, "reg");
+ DBG("clk_reg: %p", gpu->clk_reg);
+ if (IS_ERR(gpu->clk_reg))
+ return PTR_ERR(gpu->clk_reg);
+
+ gpu->clk_bus = devm_clk_get_optional(dev, "bus");
+ DBG("clk_bus: %p", gpu->clk_bus);
+ if (IS_ERR(gpu->clk_bus))
+ return PTR_ERR(gpu->clk_bus);
+
+ gpu->clk_core = devm_clk_get(dev, "core");
+ DBG("clk_core: %p", gpu->clk_core);
+ if (IS_ERR(gpu->clk_core))
+ return PTR_ERR(gpu->clk_core);
+ gpu->base_rate_core = clk_get_rate(gpu->clk_core);
+
+ gpu->clk_shader = devm_clk_get_optional(dev, "shader");
+ DBG("clk_shader: %p", gpu->clk_shader);
+ if (IS_ERR(gpu->clk_shader))
+ return PTR_ERR(gpu->clk_shader);
+ gpu->base_rate_shader = clk_get_rate(gpu->clk_shader);
+
+ return 0;
+}
+
static int etnaviv_gpu_clk_enable(struct etnaviv_gpu *gpu)
{
int ret;
@@ -1880,27 +1909,9 @@ static int etnaviv_gpu_platform_probe(struct
platform_device *pdev)
}
/* Get Clocks: */
- gpu->clk_reg = devm_clk_get_optional(>dev, "reg");
- DBG("clk_reg: %p", gpu->clk_reg);
- if (IS_ERR(gpu->clk_reg))
- return PTR_ERR(gpu->clk_reg);
-
- gpu->clk_bus = devm_clk_get_optional(>dev, "bus");
- DBG("clk_bus: %p", gpu->clk_bus);
- if (IS_ERR(gpu->clk_bus))
- return PTR_ERR(gpu->clk_bus);
-
- gpu->clk_core = devm_clk_get(>dev, "core");
- DBG("clk_core: %p", gpu->clk_core);
- if (IS_ERR(gpu->clk_core))
- return PTR_ERR(gpu->clk_core);
- gpu->base_rate_core = clk_get_rate(gpu->clk_core);
-
- gpu->clk_shader = devm_clk_get_optional(>dev, "shader");
- DBG("clk_shader: %p", gpu->clk_shader);
- if (IS_ERR(gpu->clk_shader))
- return PTR_ERR(gpu->clk_shader);
- gpu->base_rate_shader = clk_get_rate(gpu->clk_shader);
+ err = etnaviv_gpu_clk_get(gpu);
+ if (err)
+ return err;
/* TODO: figure out max mapped size */
dev_set_drvdata(dev, gpu);
--
2.34.1
[etnaviv-next v14 0/8] drm/etnaviv: Add driver wrapper for vivante GPUs attached on PCI(e) device
drm/etnaviv use the component framework to bind multiple GPU cores to a virtual master, the virtual master is manually create during driver load time. This works well for various SoCs, yet there are some PCIe card has the vivante GPU cores integrated. The driver lacks the support for PCIe devices currently. Adds PCIe driver wrapper on the top of what drm/etnaviv already has, the component framework is still being used to bind subdevices, even though there is only one GPU core. But the process is going to be reversed, we create virtual platform device for each of the vivante GPU IP core shipped by the PCIe master. The PCIe master is real, bind all the virtual child to the master with component framework. v6: * Fix build issue on system without CONFIG_PCI enabled v7: * Add a separate patch for the platform driver rearrangement (Bjorn) * Switch to runtime check if the GPU is dma coherent or not (Lucas) * Add ETNAVIV_PARAM_GPU_COHERENT to allow userspace to query (Lucas) * Remove etnaviv_gpu.no_clk member (Lucas) * Fix Various typos and coding style fixed (Bjorn) v8: * Fix typos and remove unnecessary header included (Bjorn). * Add a dedicated function to create the virtual master platform device. v9: * Use PCI_VDEVICE() macro (Bjorn) * Add trivial stubs for the PCI driver (Bjorn) * Remove a redundant dev_err() usage (Bjorn) * Clean up etnaviv_pdev_probe() with etnaviv_of_first_available_node() v10: * Add one more cleanup patch * Resolve the conflict with a patch from Rob * Make the dummy PCI stub inlined * Print only if the platform is dma-coherrent V11: * Drop unnecessary changes (Lucas) * Tweak according to other reviews of v10. V12: * Create a virtual platform device for the subcomponent GPU cores * Bind all subordinate GPU cores to the real PCI master via component. V13: * Drop the non-component code path, always use the component framework to bind subcomponent GPU core. Even though there is only one core. * Defer the irq handler register. * Rebase and improve the commit message V14: * Rebase onto etnaviv-next and improve commit message. Tested with JD9230P GPU and LingJiu GP102 GPU. Sui Jingfeng (8): drm/etnaviv: Add a dedicated helper function to get various clocks drm/etnaviv: Add constructor and destructor for the etnaviv_drm_private structure drm/etnaviv: Embed struct drm_device into struct etnaviv_drm_private drm/etnaviv: Fix wrong cache property being used for vmap() drm/etnaviv: Add support for cached coherent caching mode drm/etnaviv: Replace the '>dev' with 'dev' drm/etnaviv: Allow creating subdevices and pass platform specific data drm/etnaviv: Add support for vivante GPU cores attached via PCIe device drivers/gpu/drm/etnaviv/Kconfig | 8 + drivers/gpu/drm/etnaviv/Makefile | 2 + drivers/gpu/drm/etnaviv/etnaviv_drv.c| 159 ++-- drivers/gpu/drm/etnaviv/etnaviv_drv.h| 27 +++ drivers/gpu/drm/etnaviv/etnaviv_gem.c| 22 ++- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 2 +- drivers/gpu/drm/etnaviv/etnaviv_gpu.c| 144 +- drivers/gpu/drm/etnaviv/etnaviv_gpu.h| 4 + drivers/gpu/drm/etnaviv/etnaviv_mmu.c| 4 +- drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c| 187 +++ drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h| 18 ++ include/uapi/drm/etnaviv_drm.h | 1 + 12 files changed, 468 insertions(+), 110 deletions(-) create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h base-commit: 52272bfff15ee70c7bd5be9368f175948fb8ecfd -- 2.34.1
Re: [PATCH] drm/nouveau/nvif: Avoid build error due to potential integer overflows
On 5/18/24 18:19, Joe Perches wrote:
On Sat, 2024-05-18 at 11:23 -0700, Guenter Roeck wrote:
On 5/18/24 10:32, Kees Cook wrote:
[]
I think the INT_MAX test is actually better in this case because
nvif_object_ioctl()'s size argument is u32:
ret = nvif_object_ioctl(object, args, sizeof(*args) + size, NULL);
So that could wrap around, even though the allocation may not.
Better yet, since "sizeof(*args) + size" is repeated 3 times in the
function, I'd recommend:
...
u32 args_size;
if (check_add_overflow(sizeof(*args), size, _size))
return -ENOMEM;
if (args_size > sizeof(stack)) {
if (!(args = kmalloc(args_size, GFP_KERNEL)))
trivia:
More typical kernel style would use separate alloc and test
args = kmalloc(args_size, GFP_KERNEL);
if (!args)
Sure, I can do that as well. I'll wait a couple of days though before
sending v3 in case there are more change requests.
Guenter
[PATCH 2/2] tests/amdgpu/amd_abm: Add support for panel_power_saving property
From: Mario Limonciello
When the "panel power saving" property is set to forbidden the
compositor has indicated that userspace prefers to have color
accuracy and fidelity instead of power saving.
Verify that the sysfs file behaves as expected in this situation.
Signed-off-by: Mario Limonciello
---
tests/amdgpu/amd_abm.c | 65 ++
1 file changed, 65 insertions(+)
diff --git a/tests/amdgpu/amd_abm.c b/tests/amdgpu/amd_abm.c
index f74c3012c..971cda153 100644
--- a/tests/amdgpu/amd_abm.c
+++ b/tests/amdgpu/amd_abm.c
@@ -104,6 +104,32 @@ static int backlight_write_brightness(int value)
return 0;
}
+static int toggle_panel_power_saving_prop(data_t *data, igt_output_t *output,
bool forbid)
+{
+ uint32_t type = DRM_MODE_OBJECT_CONNECTOR;
+ bool prop_exists;
+ uint32_t prop_id;
+
+ prop_exists = kmstest_get_property(
+ data->drm_fd, output->id, type, "panel power saving",
+ _id, NULL, NULL);
+
+ if (!prop_exists)
+ return -ENODEV;
+
+ return drmModeObjectSetProperty(data->drm_fd, output->id, type,
prop_id, forbid);
+}
+
+static int allow_panel_power_saving(data_t *data, igt_output_t *output)
+{
+ return toggle_panel_power_saving_prop(data, output, false);
+}
+
+static int forbid_panel_power_saving(data_t *data, igt_output_t *output)
+{
+ return toggle_panel_power_saving_prop(data, output, true);
+}
+
static int set_abm_level(data_t *data, igt_output_t *output, int level)
{
char buf[PATH_MAX];
@@ -365,6 +391,43 @@ static void abm_gradual(data_t *data)
}
}
+
+static void abm_forbidden(data_t *data)
+{
+ igt_output_t *output;
+ enum pipe pipe;
+ int target, r;
+
+ for_each_pipe_with_valid_output(>display, pipe, output) {
+ if (output->config.connector->connector_type !=
DRM_MODE_CONNECTOR_eDP)
+ continue;
+
+ r = allow_panel_power_saving(data, output);
+ if (r == -ENODEV) {
+ igt_skip("No panel power saving prop\n");
+ return;
+ }
+ igt_assert_eq(r, 0);
+
+ target = 3;
+ r = set_abm_level(data, output, target);
+ igt_assert_eq(r, 0);
+
+ r = forbid_panel_power_saving(data, output);
+ igt_assert_eq(r, 0);
+
+ target = 0;
+ r = set_abm_level(data, output, target);
+ igt_assert_eq(r, -1);
+
+ r = allow_panel_power_saving(data, output);
+ igt_assert_eq(r, 0);
+
+ r = set_abm_level(data, output, target);
+ igt_assert_eq(r, 0);
+ }
+}
+
igt_main
{
data_t data = {};
@@ -393,6 +456,8 @@ igt_main
abm_enabled();
igt_subtest("abm_gradual")
abm_gradual();
+ igt_subtest("abm_forbidden")
+ abm_forbidden();
igt_fixture {
igt_display_fini();
--
2.45.0
[PATCH 0/2] Add support for testing panel power saving DRM property
During the Display Next hackfest 2024 one of the topics discussed was the need for compositor to be able to relay intention to drivers that color fidelity is preferred over power savings. To accomplish this a new optional DRM property is being introduced called "panel power saving". This property is an enum that can be configured by the compositor to "Allowed" or "Forbidden". When a driver advertises support for this property and the compositor sets it to "Forbidden" then the driver will disable any power saving features that can compromise color fidelity. This set of IGT changes introduces a new subtest that will cover the expected kernel behavior when switching between Allowed and Forbidden. Mario Limonciello (2): tests/amdgpu/amd_abm: Make set_abm_level return type int tests/amdgpu/amd_abm: Add support for panel_power_saving property tests/amdgpu/amd_abm.c | 98 +- 1 file changed, 88 insertions(+), 10 deletions(-) -- 2.45.0
[PATCH 1/2] tests/amdgpu/amd_abm: Make set_abm_level return type int
From: Mario Limonciello
In order to bubble of cases of expeted errors on set_abm_level()
change the return type to int.
Signed-off-by: Mario Limonciello
---
tests/amdgpu/amd_abm.c | 33 +++--
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/tests/amdgpu/amd_abm.c b/tests/amdgpu/amd_abm.c
index 2882c2c18..f74c3012c 100644
--- a/tests/amdgpu/amd_abm.c
+++ b/tests/amdgpu/amd_abm.c
@@ -104,10 +104,11 @@ static int backlight_write_brightness(int value)
return 0;
}
-static void set_abm_level(data_t *data, igt_output_t *output, int level)
+static int set_abm_level(data_t *data, igt_output_t *output, int level)
{
char buf[PATH_MAX];
int fd;
+ int ret;
igt_assert(snprintf(buf, PATH_MAX, PANEL_POWER_SAVINGS_PATH,
output->name) < PATH_MAX);
@@ -116,8 +117,12 @@ static void set_abm_level(data_t *data, igt_output_t
*output, int level)
igt_assert(fd != -1);
- igt_assert_eq(snprintf(buf, sizeof(buf), "%d", level),
- write(fd, buf, 1));
+ snprintf(buf, sizeof(buf), "%d", level);
+ ret = write(fd, buf, 1);
+ if (ret < 0) {
+ close(fd);
+ return ret;
+ }
igt_assert_eq(close(fd), 0);
@@ -129,6 +134,7 @@ static void set_abm_level(data_t *data, igt_output_t
*output, int level)
DRM_MODE_DPMS_OFF);
kmstest_set_connector_dpms(data->drm_fd, output->config.connector,
DRM_MODE_DPMS_ON);
+ return 0;
}
static int backlight_read_max_brightness(int *result)
@@ -192,7 +198,8 @@ static void backlight_dpms_cycle(data_t *data)
ret = backlight_read_max_brightness(_brightness);
igt_assert_eq(ret, 0);
- set_abm_level(data, output, 0);
+ ret = set_abm_level(data, output, 0);
+ igt_assert_eq(ret, 0);
backlight_write_brightness(max_brightness / 2);
usleep(10);
pwm_1 = read_target_backlight_pwm(data->drm_fd, output->name);
@@ -223,7 +230,8 @@ static void backlight_monotonic_basic(data_t *data)
brightness_step = max_brightness / 10;
- set_abm_level(data, output, 0);
+ ret = set_abm_level(data, output, 0);
+ igt_assert_eq(ret, 0);
backlight_write_brightness(max_brightness);
usleep(10);
prev_pwm = read_target_backlight_pwm(data->drm_fd,
output->name);
@@ -257,7 +265,8 @@ static void backlight_monotonic_abm(data_t *data)
brightness_step = max_brightness / 10;
for (i = 1; i < 5; i++) {
- set_abm_level(data, output, 0);
+ ret = set_abm_level(data, output, 0);
+ igt_assert_eq(ret, 0);
backlight_write_brightness(max_brightness);
usleep(10);
prev_pwm = read_target_backlight_pwm(data->drm_fd,
output->name);
@@ -289,14 +298,16 @@ static void abm_enabled(data_t *data)
ret = backlight_read_max_brightness(_brightness);
igt_assert_eq(ret, 0);
- set_abm_level(data, output, 0);
+ ret = set_abm_level(data, output, 0);
+ igt_assert_eq(ret, 0);
backlight_write_brightness(max_brightness);
usleep(10);
prev_pwm = read_target_backlight_pwm(data->drm_fd,
output->name);
pwm_without_abm = prev_pwm;
for (i = 1; i < 5; i++) {
- set_abm_level(data, output, i);
+ ret = set_abm_level(data, output, i);
+ igt_assert_eq(ret, 0);
usleep(10);
pwm = read_target_backlight_pwm(data->drm_fd,
output->name);
igt_assert(pwm <= prev_pwm);
@@ -323,7 +334,8 @@ static void abm_gradual(data_t *data)
igt_assert_eq(ret, 0);
- set_abm_level(data, output, 0);
+ ret = set_abm_level(data, output, 0);
+ igt_assert_eq(ret, 0);
backlight_write_brightness(max_brightness);
sleep(convergence_delay);
@@ -331,7 +343,8 @@ static void abm_gradual(data_t *data)
curr = read_current_backlight_pwm(data->drm_fd, output->name);
igt_assert_eq(prev_pwm, curr);
- set_abm_level(data, output, 4);
+ ret = set_abm_level(data, output, 4);
+ igt_assert_eq(ret, 0);
for (i = 0; i < 10; i++) {
usleep(10);
pwm = read_current_backlight_pwm(data->drm_fd,
output->name);
--
2.45.0
[PATCH 0/2] Add support for Panel Power Savings property
During the Display Next hackfest 2024 one of the topics discussed was the need for compositor to be able to relay intention to drivers that color fidelity is preferred over power savings. To accomplish this a new optional DRM property is being introduced called "panel power saving". This property is an enum that can be configured by the compositor to "Allowed" or "Forbidden". When a driver advertises support for this property and the compositor sets it to "Forbidden" then the driver will disable any power saving features that can compromise color fidelity. In practice the main feature this currently applies to is the "Adaptive Backlight Modulation" feature within AMD DCN on eDP panels. When the compositor has marked the property "Forbidden" then this feature will be disabled and any userspace that tries to turn it on will get an -EBUSY return code. When the compositor has restored the value back to "Allowed" then the previous value that would have been programmed will be restored. Mario Limonciello (2): drm: Introduce panel_power_saving drm property drm/amd: Add panel_power_saving drm property to eDP connectors drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 3 ++ .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 34 ++--- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 1 + drivers/gpu/drm/drm_connector.c | 37 +++ include/drm/drm_connector.h | 1 + include/drm/drm_mode_config.h | 6 +++ include/uapi/drm/drm_mode.h | 4 ++ 7 files changed, 81 insertions(+), 5 deletions(-) -- 2.45.0
[PATCH 2/2] drm/amd: Add panel_power_saving drm property to eDP connectors
When the `panel_power_saving` property is set to "Forbidden" ABM
should be disabled immediately and any requests by sysfs to update
will return an -EBUSY error.
When the property is restored to "Allowed" the previous value of
ABM will be restored.
Signed-off-by: Mario Limonciello
---
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 3 ++
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 36 ---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 1 +
3 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 3ecc7ef95172..6e6531c93d81 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -1350,6 +1350,9 @@ int amdgpu_display_modeset_create_props(struct
amdgpu_device *adev)
"dither",
amdgpu_dither_enum_list, sz);
+ if (adev->dc_enabled)
+ drm_mode_create_panel_power_saving_property(adev_to_drm(adev));
+
return 0;
}
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 01b0a331e4a6..f6b80018b136 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6421,6 +6421,12 @@ int amdgpu_dm_connector_atomic_set_property(struct
drm_connector *connector,
} else if (property == adev->mode_info.underscan_property) {
dm_new_state->underscan_enable = val;
ret = 0;
+ } else if (property == dev->mode_config.panel_power_saving) {
+ dm_new_state->abm_forbidden = val;
+ dm_new_state->abm_level = (val || !amdgpu_dm_abm_level) ?
+ ABM_LEVEL_IMMEDIATE_DISABLE :
+ amdgpu_dm_abm_level;
+ ret = 0;
}
return ret;
@@ -6463,6 +6469,9 @@ int amdgpu_dm_connector_atomic_get_property(struct
drm_connector *connector,
} else if (property == adev->mode_info.underscan_property) {
*val = dm_state->underscan_enable;
ret = 0;
+ } else if (property == dev->mode_config.panel_power_saving) {
+ *val = dm_state->abm_forbidden;
+ ret = 0;
}
return ret;
@@ -6489,9 +6498,12 @@ static ssize_t panel_power_savings_show(struct device
*device,
u8 val;
drm_modeset_lock(>mode_config.connection_mutex, NULL);
- val = to_dm_connector_state(connector->state)->abm_level ==
- ABM_LEVEL_IMMEDIATE_DISABLE ? 0 :
- to_dm_connector_state(connector->state)->abm_level;
+ if (to_dm_connector_state(connector->state)->abm_forbidden)
+ val = 0;
+ else
+ val = to_dm_connector_state(connector->state)->abm_level ==
+ ABM_LEVEL_IMMEDIATE_DISABLE ? 0 :
+ to_dm_connector_state(connector->state)->abm_level;
drm_modeset_unlock(>mode_config.connection_mutex);
return sysfs_emit(buf, "%u\n", val);
@@ -6515,10 +6527,16 @@ static ssize_t panel_power_savings_store(struct device
*device,
return -EINVAL;
drm_modeset_lock(>mode_config.connection_mutex, NULL);
- to_dm_connector_state(connector->state)->abm_level = val ?:
- ABM_LEVEL_IMMEDIATE_DISABLE;
+ if (to_dm_connector_state(connector->state)->abm_forbidden)
+ ret = -EBUSY;
+ else
+ to_dm_connector_state(connector->state)->abm_level = val ?:
+ ABM_LEVEL_IMMEDIATE_DISABLE;
drm_modeset_unlock(>mode_config.connection_mutex);
+ if (ret)
+ return ret;
+
drm_kms_helper_hotplug_event(dev);
return count;
@@ -7689,6 +7707,14 @@ void amdgpu_dm_connector_init_helper(struct
amdgpu_display_manager *dm,
aconnector->base.state->max_bpc = 16;
aconnector->base.state->max_requested_bpc =
aconnector->base.state->max_bpc;
+ if (connector_type == DRM_MODE_CONNECTOR_eDP &&
+ (dc_is_dmcu_initialized(adev->dm.dc) ||
+adev->dm.dc->ctx->dmub_srv) && amdgpu_dm_abm_level < 0) {
+ drm_object_attach_property(>base.base,
+ dm->ddev->mode_config.panel_power_saving,
+ DRM_MODE_PANEL_POWER_SAVING_ALLOWED);
+ }
+
if (connector_type == DRM_MODE_CONNECTOR_HDMIA) {
/* Content Type is currently only implemented for HDMI. */
drm_connector_attach_content_type_property(>base);
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index 09519b7abf67..43c3e5845a94 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++
[PATCH 1/2] drm: Introduce panel_power_saving drm property
The `panel_power_saving` DRM property is an optional property that
can be added to a connector by a driver.
This property is for compositors to indicate intent of allowing
policy for the driver to use power saving features that may
compromise color fidelity.
Signed-off-by: Mario Limonciello
---
drivers/gpu/drm/drm_connector.c | 36 +
include/drm/drm_connector.h | 1 +
include/drm/drm_mode_config.h | 6 ++
include/uapi/drm/drm_mode.h | 4
4 files changed, 47 insertions(+)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 4d2df7f64dc5..ccf672c55e0c 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -961,6 +961,11 @@ static const struct drm_prop_enum_list
drm_scaling_mode_enum_list[] = {
{ DRM_MODE_SCALE_ASPECT, "Full aspect" },
};
+static const struct drm_prop_enum_list drm_panel_power_saving_enum_list[] = {
+ { DRM_MODE_PANEL_POWER_SAVING_ALLOWED, "Allowed" },
+ { DRM_MODE_PANEL_POWER_SAVING_FORBIDDEN, "Forbidden" },
+};
+
static const struct drm_prop_enum_list drm_aspect_ratio_enum_list[] = {
{ DRM_MODE_PICTURE_ASPECT_NONE, "Automatic" },
{ DRM_MODE_PICTURE_ASPECT_4_3, "4:3" },
@@ -1963,6 +1968,37 @@ int drm_mode_create_scaling_mode_property(struct
drm_device *dev)
}
EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
+/**
+ * drm_mode_create_panel_power_saving_property - create panel power saving
property
+ * @dev: DRM device
+ *
+ * Called by a driver the first time it's needed, must be attached to desired
+ * connectors.
+ *
+ * Atomic drivers should use drm_mode_create_panel_power_saving_property()
+ * instead to correctly assign _connector_state.panel_power_saving
+ * in the atomic state.
+ *
+ * Returns: %0
+ */
+int drm_mode_create_panel_power_saving_property(struct drm_device *dev)
+{
+ struct drm_property *panel_power_saving;
+
+ if (dev->mode_config.panel_power_saving)
+ return 0;
+
+ panel_power_saving =
+ drm_property_create_enum(dev, 0, "panel power saving",
+ drm_panel_power_saving_enum_list,
+
ARRAY_SIZE(drm_panel_power_saving_enum_list));
+
+ dev->mode_config.panel_power_saving = panel_power_saving;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_mode_create_panel_power_saving_property);
+
/**
* DOC: Variable refresh properties
*
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index fe88d7fc6b8f..4ea3f912c641 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -2025,6 +2025,7 @@ int drm_mode_create_dp_colorspace_property(struct
drm_connector *connector,
u32 supported_colorspaces);
int drm_mode_create_content_type_property(struct drm_device *dev);
int drm_mode_create_suggested_offset_properties(struct drm_device *dev);
+int drm_mode_create_panel_power_saving_property(struct drm_device *dev);
int drm_connector_set_path_property(struct drm_connector *connector,
const char *path);
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 973119a9176b..099ad2d8c5c1 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -954,6 +954,12 @@ struct drm_mode_config {
*/
struct drm_atomic_state *suspend_state;
+ /**
+* @panel_power_saving: DRM ENUM property for type of
+* Panel Power Saving.
+*/
+ struct drm_property *panel_power_saving;
+
const struct drm_mode_config_helper_funcs *helper_private;
};
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 7040e7ea80c7..82e565cc76fb 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -152,6 +152,10 @@ extern "C" {
#define DRM_MODE_SCALE_CENTER 2 /* Centered, no scaling */
#define DRM_MODE_SCALE_ASPECT 3 /* Full screen, preserve aspect */
+/* Panel power saving options */
+#define DRM_MODE_PANEL_POWER_SAVING_ALLOWED0 /* Panel power savings
features allowed */
+#define DRM_MODE_PANEL_POWER_SAVING_FORBIDDEN 1 /* Panel power savings
features not allowed */
+
/* Dithering mode options */
#define DRM_MODE_DITHERING_OFF 0
#define DRM_MODE_DITHERING_ON 1
--
2.45.0
Re: [PATCH] drm/dp: Fix documentation warning
On Sun, May 19, 2024 at 12:10:27AM -0300, MarileneGarcia wrote:
> It fixes the following warnings when
> the kernel documentation is generated:
>
> ./include/drm/display/drm_dp_helper.h:126:
> warning: Function parameter or struct member
> 'mode' not described in 'drm_dp_as_sdp'
>
> ./include/drm/display/drm_dp_helper.h:126:
> warning: Excess struct member 'operation_mode'
> description in 'drm_dp_as_sdp'
>
> Signed-off-by: MarileneGarcia
> ---
> Changes:
> This documentation comment should refer to the name of the
> variable to solve the warnings. As operation_mode is the
> name of the enum, and the declared variable name is mode.
>
> Thank you.
>
> include/drm/display/drm_dp_helper.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Fixes: 0bbb8f594e33 ("drm/dp: Add Adaptive Sync SDP logging")
Reviewed-by: Dmitry Baryshkov
--
With best wishes
Dmitry
Re: [PATCH] drm/msm/adreno: Check for zap node availability
On Fri, May 17, 2024 at 12:50:19PM -0700, Rob Clark wrote: > From: Rob Clark > > This should allow disabling the zap node via an overlay, for slbounce. > > Suggested-by: Nikita Travkin > Signed-off-by: Rob Clark > --- > drivers/gpu/drm/msm/adreno/adreno_gpu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH 08/11] drm/msm/dp: switch to struct drm_edid
On Tue, May 14, 2024 at 03:55:14PM +0300, Jani Nikula wrote:
> Prefer the struct drm_edid based functions for reading the EDID and
> updating the connector.
>
> Simplify the flow by updating the EDID property when the EDID is read
> instead of at .get_modes.
>
> Signed-off-by: Jani Nikula
>
> ---
The patch looks good to me, I'd like to hear an opinion from Doug (added
to CC).
Reviewed-by: Dmitry Baryshkov
What is the merge strategy for the series? Do you plan to pick up all
the patches to drm-misc or should we pick up individual patches into
driver trees?
>
> Cc: Rob Clark
> Cc: Abhinav Kumar
> Cc: Dmitry Baryshkov
> Cc: Sean Paul
> Cc: Marijn Suijten
> Cc: linux-arm-...@vger.kernel.org
> Cc: freedr...@lists.freedesktop.org
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 11 +++
> drivers/gpu/drm/msm/dp/dp_panel.c | 47 +
> drivers/gpu/drm/msm/dp/dp_panel.h | 2 +-
> 3 files changed, 20 insertions(+), 40 deletions(-)
[skipped]
> @@ -249,10 +228,12 @@ void dp_panel_handle_sink_request(struct dp_panel
> *dp_panel)
> panel = container_of(dp_panel, struct dp_panel_private, dp_panel);
>
> if (panel->link->sink_request & DP_TEST_LINK_EDID_READ) {
> + /* FIXME: get rid of drm_edid_raw() */
The code here can get use of something like drm_edid_smth_checksum().
'Something', because I could not come up with the word that would make
it clear that it is the declared checksum instead of the actual /
computed one.
> + const struct edid *edid = drm_edid_raw(dp_panel->drm_edid);
> u8 checksum;
>
> - if (dp_panel->edid)
> - checksum = dp_panel_get_edid_checksum(dp_panel->edid);
> + if (edid)
> + checksum = dp_panel_get_edid_checksum(edid);
> else
> checksum = dp_panel->connector->real_edid_checksum;
>
--
With best wishes
Dmitry
Re: [PATCH 1/2] drm/rockchip: vop: clear DMA stop bit on flush on RK3066
On Sun, May 19, 2024 at 05:38:24AM -0300, Val Packett wrote: > > > On Sun, May 19 2024 at 09:59:47 +02:00:00, Greg KH > wrote: > > On Sun, May 19, 2024 at 04:31:31AM -0300, Val Packett wrote: > > > On the RK3066, there is a bit that must be cleared on flush, > > > otherwise > > > we do not get display output (at least for RGB). > > > > What commit id does this fix? > > I guess: f4a6de855e "drm: rockchip: vop: add rk3066 vop definitions" ? Great, can you add a Fixes: tag when you resend these? > But similar changes like: > 742203cd "drm: rockchip: add missing registers for RK3066" > 8d544233 "drm/rockchip: vop: Add directly output rgb feature for px30" > did not have any "Fixes" reference. Just because people didn't properly tag things in the past, doesn't mean you should perpetuate that problem :) thanks, greg k-h
Re: [PATCH 1/2] drm/rockchip: vop: clear DMA stop bit on flush on RK3066
On Sun, May 19 2024 at 09:59:47 +02:00:00, Greg KH wrote: On Sun, May 19, 2024 at 04:31:31AM -0300, Val Packett wrote: On the RK3066, there is a bit that must be cleared on flush, otherwise we do not get display output (at least for RGB). What commit id does this fix? I guess: f4a6de855e "drm: rockchip: vop: add rk3066 vop definitions" ? But similar changes like: 742203cd "drm: rockchip: add missing registers for RK3066" 8d544233 "drm/rockchip: vop: Add directly output rgb feature for px30" did not have any "Fixes" reference. ~val
[PATCH 1/2] drm/rockchip: vop: clear DMA stop bit on flush on RK3066
On the RK3066, there is a bit that must be cleared on flush, otherwise
we do not get display output (at least for RGB).
Signed-off-by: Val Packett
Cc: sta...@vger.kernel.org
---
Hi! This was required to get display working on an old RK3066 tablet,
along with the next tiny patch in the series enabling the RGB output.
I have spent quite a lot of time banging my head against the wall debugging
that display (especially since at the same time a scaler chip is used for
LVDS encoding), but finally adding debug prints showed that RK3066_SYS_CTRL0
ended up being reset to all-zero after being written correctly upon init.
Looking at the register definitions in the vendor driver revealed that the
reason was pretty self-explanatory: "dma_stop".
---
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 3 +++
drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 1 +
drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 1 +
3 files changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index a13473b2d..d4daeba74 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -1578,6 +1578,9 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc,
spin_lock(>reg_lock);
+ /* If the chip has a DMA stop bit (RK3066), it must be cleared. */
+ VOP_REG_SET(vop, common, dma_stop, 0);
+
/* Enable AFBC if there is some AFBC window, disable otherwise. */
s = to_rockchip_crtc_state(crtc->state);
VOP_AFBC_SET(vop, enable, s->enable_afbc);
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
index b33e5bdc2..0cf512cc1 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
@@ -122,6 +122,7 @@ struct vop_common {
struct vop_reg lut_buffer_index;
struct vop_reg gate_en;
struct vop_reg mmu_en;
+ struct vop_reg dma_stop;
struct vop_reg out_mode;
struct vop_reg standby;
};
diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
index b9ee02061..9bcb40a64 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
@@ -466,6 +466,7 @@ static const struct vop_output rk3066_output = {
};
static const struct vop_common rk3066_common = {
+ .dma_stop = VOP_REG(RK3066_SYS_CTRL0, 0x1, 0),
.standby = VOP_REG(RK3066_SYS_CTRL0, 0x1, 1),
.out_mode = VOP_REG(RK3066_DSP_CTRL0, 0xf, 0),
.cfg_done = VOP_REG(RK3066_REG_CFG_DONE, 0x1, 0),
--
2.45.0
[PATCH 2/2] drm/rockchip: vop: enable VOP_FEATURE_INTERNAL_RGB on RK3066
Signed-off-by: Val Packett
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
index 9bcb40a64..e2c6ba26f 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
@@ -515,6 +515,7 @@ static const struct vop_data rk3066_vop = {
.output = _output,
.win = rk3066_vop_win_data,
.win_size = ARRAY_SIZE(rk3066_vop_win_data),
+ .feature = VOP_FEATURE_INTERNAL_RGB,
.max_output = { 1920, 1080 },
};
--
2.45.0
[PATCH] drm/dp: Fix documentation warning
It fixes the following warnings when
the kernel documentation is generated:
./include/drm/display/drm_dp_helper.h:126:
warning: Function parameter or struct member
'mode' not described in 'drm_dp_as_sdp'
./include/drm/display/drm_dp_helper.h:126:
warning: Excess struct member 'operation_mode'
description in 'drm_dp_as_sdp'
Signed-off-by: MarileneGarcia
---
Changes:
This documentation comment should refer to the name of the
variable to solve the warnings. As operation_mode is the
name of the enum, and the declared variable name is mode.
Thank you.
include/drm/display/drm_dp_helper.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/drm/display/drm_dp_helper.h
b/include/drm/display/drm_dp_helper.h
index 8bed890eec2c..8defcc399f42 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -112,7 +112,7 @@ struct drm_dp_vsc_sdp {
* @target_rr: Target Refresh
* @duration_incr_ms: Successive frame duration increase
* @duration_decr_ms: Successive frame duration decrease
- * @operation_mode: Adaptive Sync Operation Mode
+ * @mode: Adaptive Sync Operation Mode
*/
struct drm_dp_as_sdp {
unsigned char sdp_type;
--
2.34.1
Re: [PATCH] drm/msm: Add obj flags to gpu devcoredump
On Mon, May 13, 2024 at 08:51:47AM -0700, Rob Clark wrote: > From: Rob Clark > > When debugging faults, it is useful to know how the BO is mapped (cached > vs WC, gpu readonly, etc). > > Signed-off-by: Rob Clark > --- > drivers/gpu/drm/msm/adreno/adreno_gpu.c | 1 + > drivers/gpu/drm/msm/msm_gpu.c | 6 -- > drivers/gpu/drm/msm/msm_gpu.h | 1 + > 3 files changed, 6 insertions(+), 2 deletions(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [RFC PATCH 4/4] drm/msm: switch msm_kms to use msm_iommu_disp_new()
On Fri, May 17, 2024 at 04:37:59PM -0700, Abhinav Kumar wrote: > Switch msm_kms to use msm_iommu_disp_new() so that the newly > registered fault handler will kick-in during any mmu faults. > > Signed-off-by: Abhinav Kumar > --- > drivers/gpu/drm/msm/msm_kms.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/msm_kms.c b/drivers/gpu/drm/msm/msm_kms.c > index 62c8e6163e81..1859efbbff1d 100644 > --- a/drivers/gpu/drm/msm/msm_kms.c > +++ b/drivers/gpu/drm/msm/msm_kms.c > @@ -181,7 +181,7 @@ struct msm_gem_address_space *msm_kms_init_aspace(struct > drm_device *dev) > else > iommu_dev = mdss_dev; > > - mmu = msm_iommu_new(iommu_dev, 0); > + mmu = msm_iommu_disp_new(iommu_dev, 0); > if (IS_ERR(mmu)) > return ERR_CAST(mmu); Reviewed-by: Dmitry Baryshkov Note to myself: make mdp4 use msm_kms_init_aspace(). -- With best wishes Dmitry
Re: [RFC PATCH 1/4] drm/msm: register a fault handler for display mmu faults
On Fri, May 17, 2024 at 04:37:56PM -0700, Abhinav Kumar wrote:
> In preparation to register a iommu fault handler for display
> related modules, register a fault handler for the backing
> mmu object of msm_kms.
>
> Currently, the fault handler only captures the display snapshot
> but we can expand this later if more information needs to be
> added to debug display mmu faults.
>
> Signed-off-by: Abhinav Kumar
> ---
> drivers/gpu/drm/msm/msm_kms.c | 25 +
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/msm_kms.c b/drivers/gpu/drm/msm/msm_kms.c
> index af6a6fcb1173..62c8e6163e81 100644
> --- a/drivers/gpu/drm/msm/msm_kms.c
> +++ b/drivers/gpu/drm/msm/msm_kms.c
> @@ -200,6 +200,28 @@ struct msm_gem_address_space *msm_kms_init_aspace(struct
> drm_device *dev)
> return aspace;
> }
>
> +static int msm_kms_fault_handler(void *arg, unsigned long iova, int flags,
> void *data)
> +{
> + struct msm_kms *kms = arg;
> + struct msm_disp_state *state;
> + int ret;
> +
> + ret = mutex_lock_interruptible(>dump_mutex);
> + if (ret)
> + return ret;
> +
> + state = msm_disp_snapshot_state_sync(kms);
> +
> + mutex_unlock(>dump_mutex);
> +
> + if (IS_ERR(state)) {
> + DRM_DEV_ERROR(kms->dev->dev, "failed to capture snapshot\n");
> + return PTR_ERR(state);
> + }
> +
> + return 0;
Hmm, after reading the rest of the code, this means that we won't get
the error on the console. Could you please change this to -ENOSYS?
> +}
> +
> void msm_drm_kms_uninit(struct device *dev)
> {
> struct platform_device *pdev = to_platform_device(dev);
> @@ -261,6 +283,9 @@ int msm_drm_kms_init(struct device *dev, const struct
> drm_driver *drv)
> goto err_msm_uninit;
> }
>
> + if (kms->aspace)
> + msm_mmu_set_fault_handler(kms->aspace->mmu, kms,
> msm_kms_fault_handler);
> +
> drm_helper_move_panel_connectors_to_head(ddev);
>
> drm_for_each_crtc(crtc, ddev) {
> --
> 2.44.0
>
--
With best wishes
Dmitry
Re: [RFC PATCH 3/4] drm/msm/iommu: introduce msm_iommu_disp_new() for msm_kms
On Fri, May 17, 2024 at 04:37:58PM -0700, Abhinav Kumar wrote:
> Introduce a new API msm_iommu_disp_new() for display use-cases.
>
> Signed-off-by: Abhinav Kumar
> ---
> drivers/gpu/drm/msm/msm_iommu.c | 28
> drivers/gpu/drm/msm/msm_mmu.h | 1 +
> 2 files changed, 29 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
> index a79cd18bc4c9..3d5c1bb4c013 100644
> --- a/drivers/gpu/drm/msm/msm_iommu.c
> +++ b/drivers/gpu/drm/msm/msm_iommu.c
> @@ -343,6 +343,19 @@ static int msm_gpu_fault_handler(struct iommu_domain
> *domain, struct device *dev
> return 0;
> }
>
> +static int msm_disp_fault_handler(struct iommu_domain *domain, struct device
> *dev,
> + unsigned long iova, int flags, void *arg)
> +{
> + struct msm_iommu *iommu = arg;
> +
> + if (iommu->base.handler)
> + return iommu->base.handler(iommu->base.arg, iova, flags, NULL);
> +
> + pr_warn_ratelimited("*** fault: iova=%16lx, flags=%d\n", iova, flags);
> +
> + return 0;
I'd say, drop pr_warn and return -ENOSYS, letting the
arm_smmu_context_fault() report the error.
> +}
> +
> static void msm_iommu_resume_translation(struct msm_mmu *mmu)
> {
> struct adreno_smmu_priv *adreno_smmu = dev_get_drvdata(mmu->dev);
> @@ -434,6 +447,21 @@ struct msm_mmu *msm_iommu_new(struct device *dev,
> unsigned long quirks)
> return >base;
> }
>
> +struct msm_mmu *msm_iommu_disp_new(struct device *dev, unsigned long quirks)
> +{
> + struct msm_iommu *iommu;
> + struct msm_mmu *mmu;
> +
> + mmu = msm_iommu_new(dev, quirks);
> + if (IS_ERR_OR_NULL(mmu))
> + return mmu;
> +
> + iommu = to_msm_iommu(mmu);
> + iommu_set_fault_handler(iommu->domain, msm_disp_fault_handler, iommu);
> +
> + return mmu;
> +}
> +
> struct msm_mmu *msm_iommu_gpu_new(struct device *dev, struct msm_gpu *gpu,
> unsigned long quirks)
> {
> struct adreno_smmu_priv *adreno_smmu = dev_get_drvdata(dev);
> diff --git a/drivers/gpu/drm/msm/msm_mmu.h b/drivers/gpu/drm/msm/msm_mmu.h
> index 88af4f490881..730458d08d6b 100644
> --- a/drivers/gpu/drm/msm/msm_mmu.h
> +++ b/drivers/gpu/drm/msm/msm_mmu.h
> @@ -42,6 +42,7 @@ static inline void msm_mmu_init(struct msm_mmu *mmu, struct
> device *dev,
>
> struct msm_mmu *msm_iommu_new(struct device *dev, unsigned long quirks);
> struct msm_mmu *msm_iommu_gpu_new(struct device *dev, struct msm_gpu *gpu,
> unsigned long quirks);
> +struct msm_mmu *msm_iommu_disp_new(struct device *dev, unsigned long quirks);
>
> static inline void msm_mmu_set_fault_handler(struct msm_mmu *mmu, void *arg,
> int (*handler)(void *arg, unsigned long iova, int flags, void
> *data))
> --
> 2.44.0
>
--
With best wishes
Dmitry
Re: [RFC PATCH 2/4] drm/msm/iommu: rename msm_fault_handler to msm_gpu_fault_handler
On Fri, May 17, 2024 at 04:37:57PM -0700, Abhinav Kumar wrote: > In preparation of registering a separate fault handler for > display, lets rename the existing msm_fault_handler to > msm_gpu_fault_handler. > > Signed-off-by: Abhinav Kumar > --- > drivers/gpu/drm/msm/msm_iommu.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [RFC PATCH 1/4] drm/msm: register a fault handler for display mmu faults
On Fri, May 17, 2024 at 04:37:56PM -0700, Abhinav Kumar wrote:
> In preparation to register a iommu fault handler for display
> related modules, register a fault handler for the backing
> mmu object of msm_kms.
>
> Currently, the fault handler only captures the display snapshot
> but we can expand this later if more information needs to be
> added to debug display mmu faults.
>
> Signed-off-by: Abhinav Kumar
> ---
> drivers/gpu/drm/msm/msm_kms.c | 25 +
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/msm_kms.c b/drivers/gpu/drm/msm/msm_kms.c
> index af6a6fcb1173..62c8e6163e81 100644
> --- a/drivers/gpu/drm/msm/msm_kms.c
> +++ b/drivers/gpu/drm/msm/msm_kms.c
> @@ -200,6 +200,28 @@ struct msm_gem_address_space *msm_kms_init_aspace(struct
> drm_device *dev)
> return aspace;
> }
>
> +static int msm_kms_fault_handler(void *arg, unsigned long iova, int flags,
> void *data)
> +{
> + struct msm_kms *kms = arg;
> + struct msm_disp_state *state;
> + int ret;
> +
> + ret = mutex_lock_interruptible(>dump_mutex);
> + if (ret)
> + return ret;
> +
> + state = msm_disp_snapshot_state_sync(kms);
> +
> + mutex_unlock(>dump_mutex);
> +
> + if (IS_ERR(state)) {
> + DRM_DEV_ERROR(kms->dev->dev, "failed to capture snapshot\n");
> + return PTR_ERR(state);
> + }
> +
> + return 0;
> +}
> +
> void msm_drm_kms_uninit(struct device *dev)
> {
> struct platform_device *pdev = to_platform_device(dev);
> @@ -261,6 +283,9 @@ int msm_drm_kms_init(struct device *dev, const struct
> drm_driver *drv)
> goto err_msm_uninit;
> }
>
> + if (kms->aspace)
> + msm_mmu_set_fault_handler(kms->aspace->mmu, kms,
> msm_kms_fault_handler);
> +
Can we move this to msm_kms_init_aspace() instead of checking for
kms->aspace?
> drm_helper_move_panel_connectors_to_head(ddev);
>
> drm_for_each_crtc(crtc, ddev) {
> --
> 2.44.0
>
--
With best wishes
Dmitry
Re: [RFC PATCH 0/4] drm/msm: add a display mmu fault handler
On Fri, May 17, 2024 at 04:37:55PM -0700, Abhinav Kumar wrote: > To debug display mmu faults, this series introduces a display fault > handler similar to the gpu one. > > This is only compile tested at the moment, till a suitable method > to trigger the fault is found and see if this handler does the needful > on the device. You should always be able to trigger the issue by programming wrong values to the SSPP. > > Abhinav Kumar (4): > drm/msm: register a fault handler for display mmu faults > drm/msm/iommu: rename msm_fault_handler to msm_gpu_fault_handler > drm/msm/iommu: introduce msm_iommu_disp_new() for msm_kms > drm/msm: switch msm_kms to use msm_iommu_disp_new() > > drivers/gpu/drm/msm/msm_iommu.c | 34 ++--- > drivers/gpu/drm/msm/msm_kms.c | 27 +- > drivers/gpu/drm/msm/msm_mmu.h | 1 + > 3 files changed, 58 insertions(+), 4 deletions(-) > > -- > 2.44.0 > -- With best wishes Dmitry
Re: [PATCH v4] drm/msm/a6xx: request memory region
On Sun, May 12, 2024 at 05:03:53AM -0400, Kiarash Hajian wrote:
> The driver's memory regions are currently just ioremap()ed, but not
> reserved through a request. That's not a bug, but having the request is
> a little more robust.
>
> Implement the region-request through the corresponding managed
> devres-function.
>
> Signed-off-by: Kiarash Hajian
> ---
> Changes in v4:
> - Combine v3 commits into a singel commit
> - Link to v3:
> https://lore.kernel.org/r/20240512-msm-adreno-memory-region-v3-0-0a728ad45...@gmail.com
>
> Changes in v3:
> - Remove redundant devm_iounmap calls, relying on devres for automatic
> resource cleanup.
>
> Changes in v2:
> - update the subject prefix to "drm/msm/a6xx:", to match the majority of
> other changes to this file.
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 22 +-
> 1 file changed, 1 insertion(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> index 8bea8ef26f77..cf0b3b3d8f34 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> @@ -524,9 +524,6 @@ static void a6xx_gmu_rpmh_init(struct a6xx_gmu *gmu)
> uint32_t pdc_address_offset;
> bool pdc_in_aop = false;
>
> - if (IS_ERR(pdcptr))
> - goto err;
So, if there is an error, we just continue through? What about the code
that accesses the region afterwards?
If error handling becomes void, then there should be an early return
instead of dropping the error check completely.
> -
> if (adreno_is_a650(adreno_gpu) ||
> adreno_is_a660_family(adreno_gpu) ||
> adreno_is_a7xx(adreno_gpu))
> @@ -540,8 +537,6 @@ static void a6xx_gmu_rpmh_init(struct a6xx_gmu *gmu)
>
> if (!pdc_in_aop) {
> seqptr = a6xx_gmu_get_mmio(pdev, "gmu_pdc_seq");
> - if (IS_ERR(seqptr))
> - goto err;
Same question.
> }
>
> /* Disable SDE clock gating */
> @@ -633,12 +628,6 @@ static void a6xx_gmu_rpmh_init(struct a6xx_gmu *gmu)
> wmb();
>
> a6xx_rpmh_stop(gmu);
> -
> -err:
> - if (!IS_ERR_OR_NULL(pdcptr))
> - iounmap(pdcptr);
> - if (!IS_ERR_OR_NULL(seqptr))
> - iounmap(seqptr);
> }
>
> /*
> @@ -1503,7 +1492,7 @@ static void __iomem *a6xx_gmu_get_mmio(struct
> platform_device *pdev,
> return ERR_PTR(-EINVAL);
> }
>
> - ret = ioremap(res->start, resource_size(res));
> + ret = devm_ioremap_resource(>dev, res);
> if (!ret) {
> DRM_DEV_ERROR(>dev, "Unable to map the %s registers\n",
> name);
> return ERR_PTR(-EINVAL);
> @@ -1613,13 +1602,11 @@ int a6xx_gmu_wrapper_init(struct a6xx_gpu *a6xx_gpu,
> struct device_node *node)
> gmu->mmio = a6xx_gmu_get_mmio(pdev, "gmu");
> if (IS_ERR(gmu->mmio)) {
> ret = PTR_ERR(gmu->mmio);
> - goto err_mmio;
And this is even worse. See the comment below.
> }
>
> gmu->cxpd = dev_pm_domain_attach_by_name(gmu->dev, "cx");
> if (IS_ERR(gmu->cxpd)) {
> ret = PTR_ERR(gmu->cxpd);
> - goto err_mmio;
> }
>
> if (!device_link_add(gmu->dev, gmu->cxpd, DL_FLAG_PM_RUNTIME)) {
> @@ -1635,7 +1622,6 @@ int a6xx_gmu_wrapper_init(struct a6xx_gpu *a6xx_gpu,
> struct device_node *node)
> gmu->gxpd = dev_pm_domain_attach_by_name(gmu->dev, "gx");
> if (IS_ERR(gmu->gxpd)) {
> ret = PTR_ERR(gmu->gxpd);
> - goto err_mmio;
> }
>
> gmu->initialized = true;
> @@ -1645,9 +1631,6 @@ int a6xx_gmu_wrapper_init(struct a6xx_gpu *a6xx_gpu,
> struct device_node *node)
> detach_cxpd:
> dev_pm_domain_detach(gmu->cxpd, false);
>
> -err_mmio:
> - iounmap(gmu->mmio);
> -
You have dropped the iounmap(). However now the error path should
remain. The put_device() must be called. So fix the label name and just
drop the iounmap().
> /* Drop reference taken in of_find_device_by_node */
> put_device(gmu->dev);
>
> @@ -1825,9 +1808,6 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct
> device_node *node)
> dev_pm_domain_detach(gmu->cxpd, false);
>
> err_mmio:
> - iounmap(gmu->mmio);
> - if (platform_get_resource_byname(pdev, IORESOURCE_MEM, "rscc"))
> - iounmap(gmu->rscc);
Same comment here.
> free_irq(gmu->gmu_irq, gmu);
> free_irq(gmu->hfi_irq, gmu);
>
>
> ---
> base-commit: cf87f46fd34d6c19283d9625a7822f20d90b64a4
> change-id: 20240511-msm-adreno-memory-region-2bcb1c958621
>
> Best regards,
> --
> Kiarash Hajian
>
--
With best wishes
Dmitry
Re: [PATCH 1/2] drm/rockchip: vop: clear DMA stop bit on flush on RK3066
On Sun, May 19, 2024 at 04:31:31AM -0300, Val Packett wrote: > On the RK3066, there is a bit that must be cleared on flush, otherwise > we do not get display output (at least for RGB). > > Signed-off-by: Val Packett > Cc: sta...@vger.kernel.org > --- > Hi! This was required to get display working on an old RK3066 tablet, > along with the next tiny patch in the series enabling the RGB output. > > I have spent quite a lot of time banging my head against the wall debugging > that display (especially since at the same time a scaler chip is used for > LVDS encoding), but finally adding debug prints showed that RK3066_SYS_CTRL0 > ended up being reset to all-zero after being written correctly upon init. > Looking at the register definitions in the vendor driver revealed that the > reason was pretty self-explanatory: "dma_stop". What commit id does this fix? thanks, greg k-h
Re: [PATCH 2/2] drm/rockchip: vop: enable VOP_FEATURE_INTERNAL_RGB on RK3066
On Sun, May 19, 2024 at 04:31:32AM -0300, Val Packett wrote: > Signed-off-by: Val Packett > Cc: sta...@vger.kernel.org > --- > drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 1 + > 1 file changed, 1 insertion(+) Maybe the DRM subsystem has more lax rules, but I know I can't take patches without any changelog text at all, sorry. greg k-h
Re: [PATCH 0/8] drm/panel: Some very minor err handling fixes + more _multi
On Fri, May 17, 2024 at 11:37 PM Douglas Anderson wrote: > This series is pretty much just addressing a few minor error handling > bugs that I noticed recently while reviewing some panel patches. For > the most part these are all old issues. All patches look good to me: Reviewed-by: Linus Walleij Yours, Linus Walleij
[drm-misc:topic/rust-drm 11/16] error: unreachable `pub` field
tree: git://anongit.freedesktop.org/drm/drm-misc topic/rust-drm
head: 440a8db3f583392a1a894f32782ecc397911f9af
commit: c91cc0f1abf0e3de8b46034ab2e15da4860061a7 [11/16] rust: PCI: add BAR
request and ioremap
config: x86_64-buildonly-randconfig-001-20240519
(https://download.01.org/0day-ci/archive/20240519/202405191312.68iylxvg-...@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project
617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20240519/202405191312.68iylxvg-...@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/202405191312.68iylxvg-...@intel.com/
All error/warnings (new ones prefixed by >>):
>> warning: struct `IoMem` is never constructed
--> rust/kernel/iomem.rs:10:12
|
10 | pub struct IoMem {
|^
|
= note: `#[warn(dead_code)]` on by default
--
>> warning: multiple associated items are never used
--> rust/kernel/iomem.rs:16:19
|
15 | impl IoMem {
| -- associated items in this implementation
16 | pub(crate) fn new(ioptr: usize, maxlen: usize) -> Result {
| ^^^
...
24 | fn get_io_addr(, offset: usize, len: usize) -> Result {
|^^^
...
32 | pub fn readb(, offset: usize) -> Result {
|^
...
38 | pub fn readw(, offset: usize) -> Result {
|^
...
44 | pub fn readl(, offset: usize) -> Result {
|^
...
50 | pub fn readq(, offset: usize) -> Result {
|^
...
56 | pub fn readb_relaxed(, offset: usize) -> Result {
|^
...
62 | pub fn readw_relaxed(, offset: usize) -> Result {
|^
...
68 | pub fn readl_relaxed(, offset: usize) -> Result {
|^
...
74 | pub fn readq_relaxed(, offset: usize) -> Result {
|^
...
80 | pub fn writeb(, byte: u8, offset: usize) -> Result {
|^^
...
87 | pub fn writew(, word: u16, offset: usize) -> Result {
|^^
...
94 | pub fn writel(, lword: u32, offset: usize) -> Result {
|^^
...
101 | pub fn writeq(, qword: u64, offset: usize) -> Result {
|^^
...
108 | pub fn writeb_relaxed(, byte: u8, offset: usize) -> Result {
|^^
...
115 | pub fn writew_relaxed(, word: u16, offset: usize) -> Result {
|^^
...
122 | pub fn writel_relaxed(, lword: u32, offset: usize) -> Result {
|^^
...
129 | pub fn writeq_relaxed(, qword: u64, offset: usize) -> Result {
|^^
--
>> error: unreachable `pub` field
--> rust/kernel/iomem.rs:11:5
|
11 | pub ioptr: usize,
| ---^
| |
| help: consider restricting its visibility: `pub(crate)`
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
