Re: [PATCH v6 3/5] drm: Add support to get EDID from ACPI
Hi Mario,
On Thu, Feb 15, 2024 at 8:04 PM Mario Limonciello
wrote:
> On 2/15/2024 12:47, Ville Syrjälä wrote:
> > On Thu, Feb 15, 2024 at 12:20:56PM -0600, Mario Limonciello wrote:
> >> On 2/14/2024 17:13, Ville Syrjälä wrote:
> >>> On Wed, Feb 14, 2024 at 03:57:54PM -0600, Mario Limonciello wrote:
> >>>> --- a/include/drm/drm_connector.h
> >>>> +++ b/include/drm/drm_connector.h
> >>>> @@ -1886,6 +1886,12 @@ struct drm_connector {
> >>>>
> >>>>/** @hdr_sink_metadata: HDR Metadata Information read from
> >>>> sink */
> >>>>struct hdr_sink_metadata hdr_sink_metadata;
> >>>> +
> >>>> + /**
> >>>> + * @acpi_edid_allowed: Get the EDID from the BIOS, if available.
> >>>> + * This is only applicable to eDP and LVDS displays.
> >>>> + */
> >>>> + bool acpi_edid_allowed;
> >>>
> >>> Aren't there other bools/small stuff in there for tighter packing?
> >>
> >> Does the compiler automatically do the packing if you put bools nearby
> >> in a struct? If so; TIL.
> >
> > Yes. Well, depends on the types and their alignment requirements
> > of course, and/or whether you specified __packed or not.
> >
> > You can use 'pahole' to find the holes in structures.
>
> Thanks! I don't see a __packed attribute on struct drm_connector, but
> I'll put it near by other bools in case that changes in the future.
FTR, don't add __packed unless you have a very good reason to do so.
With __packed, the compiler will emit multiple byte-accesses to
access multi-byte integrals on platforms that do not support unaligned
memory access.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 22/22] drm: ensure drm headers are self-contained and pass kernel-doc
Hi Jani,
On Thu, Mar 7, 2024 at 9:44 AM Jani Nikula wrote:
> On Thu, 07 Mar 2024, kernel test robot wrote:
> > kernel test robot noticed the following build errors:
>
> So I'm trying to make include/drm/ttm/ttm_caching.h self-contained by
> including with [1], but it fails like below.
>
> Cc: Thomas and Geert, better ideas for the include there? Looks like
> include/asm-generic/pgtable-nop4d.h isn't self-contained on m68k.
I have sent a fix
https://lore.kernel.org/r/ba359be013f379ff10f3afcea13e2f78dd9717be.1709804021.git.ge...@linux-m68k.org
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: linux-next: build failure after merge of the drm-misc tree
Hi Biju,
On Thu, Feb 22, 2024 at 9:14 AM Biju Das wrote:
> > -Original Message-
> > From: Stephen Rothwell
> > Sent: Thursday, February 22, 2024 1:46 AM
> > Subject: linux-next: build failure after merge of the drm-misc tree
> >
> > After merging the drm-misc tree, today's linux-next build (x86_64
> > allmodconfig) failed like this:
> >
> > drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c:47:6: error: redefinition of
> > 'rzg2l_du_vsp_enable'
> >47 | void rzg2l_du_vsp_enable(struct rzg2l_du_crtc *crtc)
> > | ^~~
> > In file included from drivers/gpu/drm/renesas/rz-du/rzg2l_du_drv.h:18,
> > from drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c:30:
> > drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.h:72:20: note: previous
> > definition of 'rzg2l_du_vsp_enable' with type 'void(struct rzg2l_du_crtc
> > *)'
> >72 | static inline void rzg2l_du_vsp_enable(struct rzg2l_du_crtc *crtc)
> > { };
> > |^~~
> > drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c:61:6: error: redefinition of
> > 'rzg2l_du_vsp_disable'
> >61 | void rzg2l_du_vsp_disable(struct rzg2l_du_crtc *crtc)
> > | ^~~~
> > drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.h:73:20: note: previous
> > definition of 'rzg2l_du_vsp_disable' with type 'void(struct rzg2l_du_crtc
> > *)'
> >73 | static inline void rzg2l_du_vsp_disable(struct rzg2l_du_crtc
> > *crtc) { };
> > |^~~~
> > drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c:66:6: error: redefinition of
> > 'rzg2l_du_vsp_atomic_flush'
> >66 | void rzg2l_du_vsp_atomic_flush(struct rzg2l_du_crtc *crtc)
> > | ^
> > drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.h:74:20: note: previous
> > definition of 'rzg2l_du_vsp_atomic_flush' with type 'void(struct
> > rzg2l_du_crtc *)'
> >74 | static inline void rzg2l_du_vsp_atomic_flush(struct rzg2l_du_crtc
> > *crtc) { };
> > |^
> > drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c:76:19: error: redefinition of
> > 'rzg2l_du_vsp_get_drm_plane'
> >76 | struct drm_plane *rzg2l_du_vsp_get_drm_plane(struct rzg2l_du_crtc
> > *crtc,
> > | ^~
> > drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.h:75:33: note: previous
> > definition of 'rzg2l_du_vsp_get_drm_plane' with type 'struct drm_plane
> > *(struct rzg2l_du_crtc *, unsigned int)'
> >75 | static inline struct drm_plane *rzg2l_du_vsp_get_drm_plane(struct
> > rzg2l_du_crtc *crtc,
> > | ^~
> > drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c:302:5: error: redefinition of
> > 'rzg2l_du_vsp_init'
> > 302 | int rzg2l_du_vsp_init(struct rzg2l_du_vsp *vsp, struct device_node
> > *np,
> > | ^
> > drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.h:66:19: note: previous
> > definition of 'rzg2l_du_vsp_init' with type 'int(struct rzg2l_du_vsp *,
> > struct device_node *, unsigned int)'
> >66 | static inline int rzg2l_du_vsp_init(struct rzg2l_du_vsp *vsp,
> > struct device_node *np,
> > | ^
> >
> > Caused by commit
> >
> > 768e9e61b3b9 ("drm: renesas: Add RZ/G2L DU Support")
> >
> > I have used the drm-misc tree from next-20240221 for today.
>
> I will send an incremental patch to fix this build error with x86 on drm-next.
>
> I need to use the macro #if IS_ENABLED(CONFIG_VIDEO_RENESAS_VSP1)
> in drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.h to fix this error.
Looks like you're also missing an EXPORT_SYMBOL_GPL(rzg2l_du_vsp_enable)?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: Re: [PULL] drm-misc-fixes
Hi Maxime, Dave,
On Thu, Feb 15, 2024 at 5:45 PM Geert Uytterhoeven wrote:
> On Thu, Feb 15, 2024 at 5:09 PM Maxime Ripard wrote:
> On Thu, Feb 15, 2024 at 01:41:24PM +0100, Geert Uytterhoeven wrote:
> > > On Thu, 15 Feb 2024, Maxime Ripard wrote:
> > > > Matthew Auld (1):
> > > > drm/tests/drm_buddy: add alloc_contiguous test
> > >
> > > Please drop this one.
> > >
> > > nore...@ellerman.id.au reported a build failure on m68k (and presumably
> > > other 32-bit platforms) in next-20240215:
> > >
> > > ERROR: modpost: "__umoddi3" [drivers/gpu/drm/tests/drm_buddy_test.ko]
> > > undefined!
> > > ERROR: modpost: "__moddi3" [drivers/gpu/drm/tests/drm_buddy_test.ko]
> > > undefined!
> > >
> > > Reverting commit a64056bb5a3215bd ("drm/tests/drm_buddy: add
> > > alloc_contiguous test") fixes the issue.
> >
> > From a quick cross-compile test with arm(32), it seems to work there
> > interestingly:
> >
> > ./tools/testing/kunit/kunit.py run \
> > --kunitconfig=drivers/gpu/drm//tests \
> > --cross_compile arm-linux-gnu- --arch arm
>
> shmobile_defconfig + CONFIG_DRM_KUNIT_TEST=y + CONFIG_KUNIT=y:
>
> arm-linux-gnueabihf-ld: drivers/gpu/drm/tests/drm_buddy_test.o: in
> function `drm_test_buddy_alloc_contiguous':
> drm_buddy_test.c:(.text+0xe0): undefined reference to `__aeabi_uldivmod'
>
> > But I agree, with should wait for a fix or a revert before merging this.
>
> Great, thanks!
Unfortunately the breakage still made it upstream.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: Re: [PULL] drm-misc-fixes
Hi Maxime,
On Thu, Feb 15, 2024 at 5:09 PM Maxime Ripard wrote:
On Thu, Feb 15, 2024 at 01:41:24PM +0100, Geert Uytterhoeven wrote:
> > On Thu, 15 Feb 2024, Maxime Ripard wrote:
> > > Matthew Auld (1):
> > > drm/tests/drm_buddy: add alloc_contiguous test
> >
> > Please drop this one.
> >
> > nore...@ellerman.id.au reported a build failure on m68k (and presumably
> > other 32-bit platforms) in next-20240215:
> >
> > ERROR: modpost: "__umoddi3" [drivers/gpu/drm/tests/drm_buddy_test.ko]
> > undefined!
> > ERROR: modpost: "__moddi3" [drivers/gpu/drm/tests/drm_buddy_test.ko]
> > undefined!
> >
> > Reverting commit a64056bb5a3215bd ("drm/tests/drm_buddy: add
> > alloc_contiguous test") fixes the issue.
>
> From a quick cross-compile test with arm(32), it seems to work there
> interestingly:
>
> ./tools/testing/kunit/kunit.py run \
> --kunitconfig=drivers/gpu/drm//tests \
> --cross_compile arm-linux-gnu- --arch arm
shmobile_defconfig + CONFIG_DRM_KUNIT_TEST=y + CONFIG_KUNIT=y:
arm-linux-gnueabihf-ld: drivers/gpu/drm/tests/drm_buddy_test.o: in
function `drm_test_buddy_alloc_contiguous':
drm_buddy_test.c:(.text+0xe0): undefined reference to `__aeabi_uldivmod'
> But I agree, with should wait for a fix or a revert before merging this.
Great, thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PULL] drm-misc-fixes
Hi Maxime,
On Thu, 15 Feb 2024, Maxime Ripard wrote:
Here's this week drm-misc-fixes PR
Maxime
drm-misc-fixes-2024-02-15:
A suspend/resume error fix for ivpu, a couple of scheduler fixes for
nouveau, a patch to support large page arrays in prime, a uninitialized
variable fix in crtc, a locking fix in rockchip/vop2 and a buddy
allocator error reporting fix.
The following changes since commit 5f8408aca66772d3aa9b4831577b2ac5ec41bcd9:
accel/ivpu: Add job status for jobs aborted by the driver (2024-02-06 13:37:34
+0100)
are available in the Git repository at:
git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-fixes-2024-02-15
for you to fetch changes up to a64056bb5a3215bd31c8ce17d609ba0f4d5c55ea:
drm/tests/drm_buddy: add alloc_contiguous test (2024-02-14 15:22:21 +0100)
A suspend/resume error fix for ivpu, a couple of scheduler fixes for
nouveau, a patch to support large page arrays in prime, a uninitialized
variable fix in crtc, a locking fix in rockchip/vop2 and a buddy
allocator error reporting fix.
Matthew Auld (1):
drm/tests/drm_buddy: add alloc_contiguous test
Please drop this one.
nore...@ellerman.id.au reported a build failure on m68k (and presumably
other 32-bit platforms) in next-20240215:
ERROR: modpost: "__umoddi3" [drivers/gpu/drm/tests/drm_buddy_test.ko] undefined!
ERROR: modpost: "__moddi3" [drivers/gpu/drm/tests/drm_buddy_test.ko] undefined!
Reverting commit a64056bb5a3215bd ("drm/tests/drm_buddy: add
alloc_contiguous test") fixes the issue.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Intel-gfx] linux-next: manual merge of the drm tree with the drm-misc-fixes tree
Hi Stephen,
On Wed, Nov 22, 2023 at 1:29 AM Stephen Rothwell wrote:
> Today's linux-next merge of the drm tree got a conflict in:
>
> drivers/accel/ivpu/ivpu_hw_37xx.c
>
> between commit:
>
> 3f7c0634926d ("accel/ivpu/37xx: Fix hangs related to MMIO reset")
>
> from the drm-misc-fixes tree and commits:
>
> 3de6d9597892 ("accel/ivpu: Pass D0i3 residency time to the VPU firmware")
> cc19fedab8bd ("accel/ivpu/37xx: Print warning when VPUIP is not idle during
> power down")
>
> from the drm tree.
>
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging. You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
Thanks for your resolution!
> --- a/drivers/accel/ivpu/ivpu_hw_37xx.c
> +++ b/drivers/accel/ivpu/ivpu_hw_37xx.c
> @@@ -720,14 -731,19 +733,14 @@@ static int ivpu_hw_37xx_power_down(stru
> {
> int ret = 0;
>
> - if (!ivpu_hw_37xx_is_idle(vdev))
> - ivpu_warn(vdev, "VPU not idle during power down\n");
> + ivpu_hw_37xx_save_d0i3_entry_timestamp(vdev);
>
> - if (ivpu_hw_37xx_reset(vdev)) {
> - ivpu_err(vdev, "Failed to reset VPU\n");
> - ret = -EIO;
> + if (!ivpu_hw_37xx_is_idle(vdev)) {
> + ivpu_warn(vdev, "VPU not idle during power down\n");
> + if (ivpu_hw_37xx_reset(vdev))
> + ivpu_warn(vdev, "Failed to reset the VPU\n");
> }
>
> - if (ivpu_pll_disable(vdev)) {
> - ivpu_err(vdev, "Failed to disable PLL\n");
> - ret = -EIO;
> - }
> -
> if (ivpu_hw_37xx_d0i3_enable(vdev)) {
> ivpu_err(vdev, "Failed to enter D0I3\n");
> ret = -EIO;
I've just run into the same conflict, and I think you lost the split
into two if-statements in the last hunk of commit 3f7c0634926d
("accel/ivpu/37xx: Fix hangs related to MMIO reset")?
My resolution is:
--- a/drivers/accel/ivpu/ivpu_hw_37xx.c
+++ b/drivers/accel/ivpu/ivpu_hw_37xx.c
@@@ -720,11 -731,16 +733,13 @@@ static int ivpu_hw_37xx_power_down(stru
{
int ret = 0;
+ ivpu_hw_37xx_save_d0i3_entry_timestamp(vdev);
+
- if (!ivpu_hw_37xx_is_idle(vdev)) {
+ if (!ivpu_hw_37xx_is_idle(vdev))
ivpu_warn(vdev, "VPU not idle during power down\n");
- if (ivpu_hw_37xx_reset(vdev))
- ivpu_warn(vdev, "Failed to reset the VPU\n");
- }
- if (ivpu_pll_disable(vdev)) {
- ivpu_err(vdev, "Failed to disable PLL\n");
+ if (ivpu_hw_37xx_reset(vdev)) {
+ ivpu_err(vdev, "Failed to reset VPU\n");
ret = -EIO;
}
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[Intel-gfx] [PATCH v2] drm: Spelling s/sempahore/semaphore/
Fix misspellings of "semaphore". Signed-off-by: Geert Uytterhoeven Reviewed-by: Hamza Mahfooz --- v2: - Add Reviewed-by. --- drivers/gpu/drm/i915/i915_request.c | 2 +- drivers/gpu/drm/radeon/cik.c| 2 +- drivers/gpu/drm/radeon/r600.c | 2 +- include/drm/task_barrier.h | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 894068bb37b6f1b6..32323bb801a139b7 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -1220,7 +1220,7 @@ emit_semaphore_wait(struct i915_request *to, /* * If this or its dependents are waiting on an external fence * that may fail catastrophically, then we want to avoid using -* sempahores as they bypass the fence signaling metadata, and we +* semaphores as they bypass the fence signaling metadata, and we * lose the fence->error propagation. */ if (from->sched.flags & I915_SCHED_HAS_EXTERNAL_CHAIN) diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c index 5819737c21c678d3..5d6b81a6578ef2ba 100644 --- a/drivers/gpu/drm/radeon/cik.c +++ b/drivers/gpu/drm/radeon/cik.c @@ -3603,7 +3603,7 @@ void cik_fence_compute_ring_emit(struct radeon_device *rdev, * @rdev: radeon_device pointer * @ring: radeon ring buffer object * @semaphore: radeon semaphore object - * @emit_wait: Is this a sempahore wait? + * @emit_wait: Is this a semaphore wait? * * Emits a semaphore signal/wait packet to the CP ring and prevents the PFP * from running ahead of semaphore waits. diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c index 382795a8b3c064ba..a17b95eec65fb810 100644 --- a/drivers/gpu/drm/radeon/r600.c +++ b/drivers/gpu/drm/radeon/r600.c @@ -2918,7 +2918,7 @@ void r600_fence_ring_emit(struct radeon_device *rdev, * @rdev: radeon_device pointer * @ring: radeon ring buffer object * @semaphore: radeon semaphore object - * @emit_wait: Is this a sempahore wait? + * @emit_wait: Is this a semaphore wait? * * Emits a semaphore signal/wait packet to the CP ring and prevents the PFP * from running ahead of semaphore waits. diff --git a/include/drm/task_barrier.h b/include/drm/task_barrier.h index 087e3f649c52f02d..217c1cf21c1ab7d5 100644 --- a/include/drm/task_barrier.h +++ b/include/drm/task_barrier.h @@ -25,7 +25,7 @@ /* * Reusable 2 PHASE task barrier (randevouz point) implementation for N tasks. - * Based on the Little book of sempahores - https://greenteapress.com/wp/semaphores/ + * Based on the Little book of semaphores - https://greenteapress.com/wp/semaphores/ */ -- 2.34.1
Re: [Intel-gfx] [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev
Hi Uwe,
Let's add some fuel to keep the thread alive ;-)
On Wed, Jul 12, 2023 at 6:13 PM Uwe Kleine-König
wrote:
> On Wed, Jul 12, 2023 at 05:34:28PM +0300, Jani Nikula wrote:
> > On Wed, 12 Jul 2023, Uwe Kleine-König
> > wrote:
> > > Hello,
> > >
> > > while I debugged an issue in the imx-lcdc driver I was constantly
> > > irritated about struct drm_device pointer variables being named "dev"
> > > because with that name I usually expect a struct device pointer.
> > >
> > > I think there is a big benefit when these are all renamed to "drm_dev".
> > > I have no strong preference here though, so "drmdev" or "drm" are fine
> > > for me, too. Let the bikesheding begin!
> > >
> > > Some statistics:
> > >
> > > $ git grep -ohE 'struct drm_device *\* *[^ (),;]*' v6.5-rc1 | sort | uniq
> > > -c | sort -n
> > > 1 struct drm_device *adev_to_drm
> > > 1 struct drm_device *drm_
> > > 1 struct drm_device *drm_dev
> > > 1 struct drm_device*drm_dev
> > > 1 struct drm_device *pdev
> > > 1 struct drm_device *rdev
> > > 1 struct drm_device *vdev
> > > 2 struct drm_device *dcss_drv_dev_to_drm
> > > 2 struct drm_device **ddev
> > > 2 struct drm_device *drm_dev_alloc
> > > 2 struct drm_device *mock
> > > 2 struct drm_device *p_ddev
> > > 5 struct drm_device *device
> > > 9 struct drm_device * dev
> > > 25 struct drm_device *d
> > > 95 struct drm_device *
> > > 216 struct drm_device *ddev
> > > 234 struct drm_device *drm_dev
> > > 611 struct drm_device *drm
> > >4190 struct drm_device *dev
> > >
> > > This series starts with renaming struct drm_crtc::dev to drm_dev. If
> > > it's not only me and others like the result of this effort it should be
> > > followed up by adapting the other structs and the individual usages in
> > > the different drivers.
> >
> > I think this is an unnecessary change. In drm, a dev is usually a drm
> > device, i.e. struct drm_device *.
>
> Well, unless it's not. Prominently there is
>
> struct drm_device {
> ...
> struct device *dev;
> ...
> };
>
> which yields quite a few code locations using dev->dev which is
> IMHO unnecessary irritating:
>
> $ git grep '\dev' v6.5-rc1 drivers/gpu/drm | wc -l
> 1633
I find that irritating as well...
Same for e.g. crtc->crtc.
Hence that's why I had sent patches to rename the base members in the
shmob_drm-specific subclasses of drm_{crtc,connector,plane} to "base".
https://lore.kernel.org/dri-devel/b3daca80f82625ba14e3aeaf2fca6dcefa056e47.1687423204.git.geert+rene...@glider.be
> Also the functions that deal with both a struct device and a struct
> drm_device often use "dev" for the struct device and then "ddev" for
> the drm_device (see for example amdgpu_device_get_pcie_replay_count()).
I guess you considered "drm_dev", because it is still a short name?
Code dealing with platform devices usually uses "pdev" and "dev".
Same for PCI drivers (despite "pci_dev" being a short name).
So my personal preference goes to "ddev".
EOF (End-of-Fuel ;-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Intel-gfx] [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev
Hi Jani,
On Thu, Jul 13, 2023 at 11:03 AM Jani Nikula wrote:
> On Wed, 12 Jul 2023, Uwe Kleine-König wrote:
> > On Wed, Jul 12, 2023 at 05:34:28PM +0300, Jani Nikula wrote:
> >> On Wed, 12 Jul 2023, Uwe Kleine-König
> >> wrote:
> >> > while I debugged an issue in the imx-lcdc driver I was constantly
> >> > irritated about struct drm_device pointer variables being named "dev"
> >> > because with that name I usually expect a struct device pointer.
> >> >
> >> > I think there is a big benefit when these are all renamed to "drm_dev".
> >> > I have no strong preference here though, so "drmdev" or "drm" are fine
> >> > for me, too. Let the bikesheding begin!
> >> >
> >> > Some statistics:
> >> >
> >> > $ git grep -ohE 'struct drm_device *\* *[^ (),;]*' v6.5-rc1 | sort |
> >> > uniq -c | sort -n
> >> > 1 struct drm_device *adev_to_drm
> >> > 1 struct drm_device *drm_
> >> > 1 struct drm_device *drm_dev
> >> > 1 struct drm_device*drm_dev
> >> > 1 struct drm_device *pdev
> >> > 1 struct drm_device *rdev
> >> > 1 struct drm_device *vdev
> >> > 2 struct drm_device *dcss_drv_dev_to_drm
> >> > 2 struct drm_device **ddev
> >> > 2 struct drm_device *drm_dev_alloc
> >> > 2 struct drm_device *mock
> >> > 2 struct drm_device *p_ddev
> >> > 5 struct drm_device *device
> >> > 9 struct drm_device * dev
> >> > 25 struct drm_device *d
> >> > 95 struct drm_device *
> >> > 216 struct drm_device *ddev
> >> > 234 struct drm_device *drm_dev
> >> > 611 struct drm_device *drm
> >> >4190 struct drm_device *dev
> >> >
> >> > This series starts with renaming struct drm_crtc::dev to drm_dev. If
> >> > it's not only me and others like the result of this effort it should be
> >> > followed up by adapting the other structs and the individual usages in
> >> > the different drivers.
> >>
> >> I think this is an unnecessary change. In drm, a dev is usually a drm
> >> device, i.e. struct drm_device *.
> >
> > Well, unless it's not. Prominently there is
> >
> > struct drm_device {
> > ...
> > struct device *dev;
> > ...
> > };
> >
> > which yields quite a few code locations using dev->dev which is
> > IMHO unnecessary irritating:
> >
> > $ git grep '\dev' v6.5-rc1 drivers/gpu/drm | wc -l
> > 1633
> >
> > Also the functions that deal with both a struct device and a struct
> > drm_device often use "dev" for the struct device and then "ddev" for
> > the drm_device (see for example amdgpu_device_get_pcie_replay_count()).
>
> Why is specifically struct drm_device *dev so irritating to you?
>
> You lead us to believe it's an outlier in kernel, something that goes
> against common kernel style, but it's really not:
>
> $ git grep -how "struct [A-Za-z0-9_]\+ \*dev" | sort | uniq -c | sort -rn |
> head -20
> 38494 struct device *dev
> 16388 struct net_device *dev
>4184 struct drm_device *dev
>2780 struct pci_dev *dev
>1916 struct comedi_device *dev
>1510 struct mlx5_core_dev *dev
>1057 struct mlx4_dev *dev
> 894 struct b43_wldev *dev
> 762 struct input_dev *dev
> 623 struct usbnet *dev
> 561 struct mlx5_ib_dev *dev
> 525 struct mt76_dev *dev
> 465 struct mt76x02_dev *dev
> 435 struct platform_device *dev
> 431 struct usb_device *dev
> 411 struct mt7915_dev *dev
> 398 struct cx231xx *dev
> 378 struct mei_device *dev
> 363 struct ksz_device *dev
> 359 struct mthca_dev *dev
>
> A good portion of the above also have a dev member.
Not all of them access both the foo_device and the device pointers.
Let's put the number of 435 platform_device pointers named "dev"
into perspective:
10095 struct platform_device *pdev
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Intel-gfx] [PATCH] fbmem: Reject FB_ACTIVATE_KD_TEXT from userspace
Hi Daniel,
On Tue, Apr 11, 2023 at 3:44 PM Daniel Vetter wrote:
> On Tue, Apr 04, 2023 at 09:39:34PM +0200, Daniel Vetter wrote:
> > This is an oversight from dc5bdb68b5b3 ("drm/fb-helper: Fix vt
> > restore") - I failed to realize that nasty userspace could set this.
> >
> > It's not pretty to mix up kernel-internal and userspace uapi flags
> > like this, but since the entire fb_var_screeninfo structure is uapi
> > we'd need to either add a new parameter to the ->fb_set_par callback
> > and fb_set_par() function, which has a _lot_ of users. Or some other
> > fairly ugly side-channel int fb_info. Neither is a pretty prospect.
> >
> > Instead just correct the issue at hand by filtering out this
> > kernel-internal flag in the ioctl handling code.
> >
> > Signed-off-by: Daniel Vetter
> > Fixes: dc5bdb68b5b3 ("drm/fb-helper: Fix vt restore")
> An Ack on this (or a better idea) would be great, so I can stuff it into
> -fixes.
I don't understand what the original commit this fixes is doing anyway...
> > --- a/drivers/video/fbdev/core/fbmem.c
> > +++ b/drivers/video/fbdev/core/fbmem.c
> > @@ -1116,6 +1116,8 @@ static long do_fb_ioctl(struct fb_info *info,
> > unsigned int cmd,
> > case FBIOPUT_VSCREENINFO:
> > if (copy_from_user(, argp, sizeof(var)))
> > return -EFAULT;
> > + /* only for kernel-internal use */
> > + var.activate &= ~FB_ACTIVATE_KD_TEXT;
> > console_lock();
> > lock_fb_info(info);
> > ret = fbcon_modechange_possible(info, );
Perhaps FB_ACTIVATE_KD_TEXT should be removed (marked as
reserved) from include/uapi/linux/fb.h, too?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Intel-gfx] [PATCH 2/2] drm: Spelling s/randevouz/rendez-vouz/
Hi Marc,
On Tue, Apr 11, 2023 at 12:49 PM Marc Dionne wrote:
> On Tue, Apr 11, 2023 at 7:44 AM Geert Uytterhoeven
> wrote:
> >
> > Fix a misspelling of "rendez-vouz".
> >
> > Signed-off-by: Geert Uytterhoeven
> > --- a/include/drm/task_barrier.h
> > +++ b/include/drm/task_barrier.h
> > @@ -24,7 +24,7 @@
> > #include
> >
> > /*
> > - * Reusable 2 PHASE task barrier (randevouz point) implementation for N
> > tasks.
> > + * Reusable 2 PHASE task barrier (rendez-vouz point) implementation for N
> > tasks.
> > * Based on the Little book of semaphores -
> > https://greenteapress.com/wp/semaphores/
> > */
> >
> > --
> > 2.34.1
>
> Sorry for the drive by comment, but shouldn't this be "rendez-vous"
> (with an 's' rather than a 'z')?
Yes it should. Thanks!
/me hides in a brown paper bag, under a rock...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[Intel-gfx] [PATCH v2 2/2] drm: Spelling s/randevouz/rendez-vous/
Fix a misspelling of "rendez-vous". Signed-off-by: Geert Uytterhoeven --- v2: - s/vouz/vous/. --- include/drm/task_barrier.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/task_barrier.h b/include/drm/task_barrier.h index 217c1cf21c1ab7d5..f6e6ed52968133d2 100644 --- a/include/drm/task_barrier.h +++ b/include/drm/task_barrier.h @@ -24,7 +24,7 @@ #include /* - * Reusable 2 PHASE task barrier (randevouz point) implementation for N tasks. + * Reusable 2 PHASE task barrier (rendez-vous point) implementation for N tasks. * Based on the Little book of semaphores - https://greenteapress.com/wp/semaphores/ */ -- 2.34.1
[Intel-gfx] [PATCH v2 1/2] drm: Spelling s/sempahore/semaphore/
Fix misspellings of "semaphore". Signed-off-by: Geert Uytterhoeven --- drivers/gpu/drm/i915/i915_request.c | 2 +- drivers/gpu/drm/radeon/cik.c| 2 +- drivers/gpu/drm/radeon/r600.c | 2 +- include/drm/task_barrier.h | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 630a732aaecca8fb..0bb368a5dd0bb107 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -1220,7 +1220,7 @@ emit_semaphore_wait(struct i915_request *to, /* * If this or its dependents are waiting on an external fence * that may fail catastrophically, then we want to avoid using -* sempahores as they bypass the fence signaling metadata, and we +* semaphores as they bypass the fence signaling metadata, and we * lose the fence->error propagation. */ if (from->sched.flags & I915_SCHED_HAS_EXTERNAL_CHAIN) diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c index 5819737c21c678d3..5d6b81a6578ef2ba 100644 --- a/drivers/gpu/drm/radeon/cik.c +++ b/drivers/gpu/drm/radeon/cik.c @@ -3603,7 +3603,7 @@ void cik_fence_compute_ring_emit(struct radeon_device *rdev, * @rdev: radeon_device pointer * @ring: radeon ring buffer object * @semaphore: radeon semaphore object - * @emit_wait: Is this a sempahore wait? + * @emit_wait: Is this a semaphore wait? * * Emits a semaphore signal/wait packet to the CP ring and prevents the PFP * from running ahead of semaphore waits. diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c index dd78fc4994024815..34457e51035278fb 100644 --- a/drivers/gpu/drm/radeon/r600.c +++ b/drivers/gpu/drm/radeon/r600.c @@ -2918,7 +2918,7 @@ void r600_fence_ring_emit(struct radeon_device *rdev, * @rdev: radeon_device pointer * @ring: radeon ring buffer object * @semaphore: radeon semaphore object - * @emit_wait: Is this a sempahore wait? + * @emit_wait: Is this a semaphore wait? * * Emits a semaphore signal/wait packet to the CP ring and prevents the PFP * from running ahead of semaphore waits. diff --git a/include/drm/task_barrier.h b/include/drm/task_barrier.h index 087e3f649c52f02d..217c1cf21c1ab7d5 100644 --- a/include/drm/task_barrier.h +++ b/include/drm/task_barrier.h @@ -25,7 +25,7 @@ /* * Reusable 2 PHASE task barrier (randevouz point) implementation for N tasks. - * Based on the Little book of sempahores - https://greenteapress.com/wp/semaphores/ + * Based on the Little book of semaphores - https://greenteapress.com/wp/semaphores/ */ -- 2.34.1
[Intel-gfx] [PATCH 2/2] drm: Spelling s/randevouz/rendez-vouz/
Fix a misspelling of "rendez-vouz". Signed-off-by: Geert Uytterhoeven --- include/drm/task_barrier.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/task_barrier.h b/include/drm/task_barrier.h index 217c1cf21c1ab7d5..59ead429acb2afb0 100644 --- a/include/drm/task_barrier.h +++ b/include/drm/task_barrier.h @@ -24,7 +24,7 @@ #include /* - * Reusable 2 PHASE task barrier (randevouz point) implementation for N tasks. + * Reusable 2 PHASE task barrier (rendez-vouz point) implementation for N tasks. * Based on the Little book of semaphores - https://greenteapress.com/wp/semaphores/ */ -- 2.34.1
[Intel-gfx] [PATCH 1/2] drm: Spelling s/sempahore/semaphore/
Fix misspellings of "semaphore". Signed-off-by: Geert Uytterhoeven --- drivers/gpu/drm/i915/i915_request.c | 2 +- drivers/gpu/drm/radeon/cik.c| 2 +- drivers/gpu/drm/radeon/r600.c | 2 +- include/drm/task_barrier.h | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 630a732aaecca8fb..0bb368a5dd0bb107 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -1220,7 +1220,7 @@ emit_semaphore_wait(struct i915_request *to, /* * If this or its dependents are waiting on an external fence * that may fail catastrophically, then we want to avoid using -* sempahores as they bypass the fence signaling metadata, and we +* semaphores as they bypass the fence signaling metadata, and we * lose the fence->error propagation. */ if (from->sched.flags & I915_SCHED_HAS_EXTERNAL_CHAIN) diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c index 5819737c21c678d3..5d6b81a6578ef2ba 100644 --- a/drivers/gpu/drm/radeon/cik.c +++ b/drivers/gpu/drm/radeon/cik.c @@ -3603,7 +3603,7 @@ void cik_fence_compute_ring_emit(struct radeon_device *rdev, * @rdev: radeon_device pointer * @ring: radeon ring buffer object * @semaphore: radeon semaphore object - * @emit_wait: Is this a sempahore wait? + * @emit_wait: Is this a semaphore wait? * * Emits a semaphore signal/wait packet to the CP ring and prevents the PFP * from running ahead of semaphore waits. diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c index dd78fc4994024815..34457e51035278fb 100644 --- a/drivers/gpu/drm/radeon/r600.c +++ b/drivers/gpu/drm/radeon/r600.c @@ -2918,7 +2918,7 @@ void r600_fence_ring_emit(struct radeon_device *rdev, * @rdev: radeon_device pointer * @ring: radeon ring buffer object * @semaphore: radeon semaphore object - * @emit_wait: Is this a sempahore wait? + * @emit_wait: Is this a semaphore wait? * * Emits a semaphore signal/wait packet to the CP ring and prevents the PFP * from running ahead of semaphore waits. diff --git a/include/drm/task_barrier.h b/include/drm/task_barrier.h index 087e3f649c52f02d..217c1cf21c1ab7d5 100644 --- a/include/drm/task_barrier.h +++ b/include/drm/task_barrier.h @@ -25,7 +25,7 @@ /* * Reusable 2 PHASE task barrier (randevouz point) implementation for N tasks. - * Based on the Little book of sempahores - https://greenteapress.com/wp/semaphores/ + * Based on the Little book of semaphores - https://greenteapress.com/wp/semaphores/ */ -- 2.34.1
Re: [Intel-gfx] [PATCH] drm/fb-helper: Remove helpers to change frame buffer config
Hi Daniel,
On Tue, Aug 9, 2022 at 5:03 PM Daniel Vetter wrote:
> On Sat, Jul 02, 2022 at 08:05:54PM +0200, Helge Deller wrote:
> > On 6/29/22 12:56, Geert Uytterhoeven wrote:
> > > The DRM fbdev emulation layer does not support pushing back
> > > changes to fb_var_screeninfo to KMS.
> > >
> > > However, drm_fb_helper still implements the fb_ops.fb_check_var() and
> > > fb_ops.fb_set_par() callbacks, but the former fails to validate various
> > > parameters passed from userspace. Hence unsanitized and possible
> > > invaled values are passed up through the fbdev, fbcon, and console
> > > stack, which has become an endless source of security issues reported
> > > against fbdev.
> > >
> > > Fix this by not populating the fb_ops.fb_check_var() and
> > > fb_ops.fb_set_par() callbacks, as there is no point in providing them if
> > > the frame buffer config cannot be changed anyway. This makes the fbdev
> > > core aware that making changes to the frame buffer config is not
> > > supported, so it will always return the current config.
> > >
> > > Signed-off-by: Geert Uytterhoeven
> >
> > It's unfortunate that DRM currently isn't able to switch resolutions
> > at runtime.
> >
> > With that in mind I agree with Geert that it's probably better to
> > disable (or drop) that code until DRM can cope with fbdev's
> > interface to switch resolutions.
> >
> > Furthermore, with the upcoming patches to fbcon (which prevents crashes on
> > invalid userspace input), you will face a kernel WARNING if you call fbset
> > to switch screen resolutions with DRM drivers.
> >
> > So, from my side (although I'd prefer a better fix for DRM):
> >
> > Acked-by: Helge Deller
>
> So maybe I'm missing something, but I think this breaks a lot of stuff.
> The issue is that fbdev is only a subordinate owner of the kms device, if
> there's a real drm kms owner around that wins.
>
> Which means when you switch back then set_par needs to restore the fbdev
> framebuffer. So that might break some recovery/use-cases.
Upon closer look, I think I was a bit too over-eager, and we can keep
the implementation of fb_ops.fb_set_par().
> The other thing is that I think this also breaks the scanout offset, and
> people do use that for double-buffering on top of fbdev on top of drm
> drivers. So we can't just nuke it completely.
You mean panning? That does not need fb_ops.fb_check_var(), as it
should be done through fb_ops.fb_pan_display().
> For better or worse I think we need to keep playing the whack-a-mole game.
> Or do I miss something here?
With the above fixed, we can continue whacking the drm bugs in
implementing the fbdev API?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Intel-gfx] [PATCH v3 2/2] drm/probe_helper: sort out poll_running vs poll_enabled
Hi Dmitry,
On Tue, 24 Jan 2023, Dmitry Baryshkov wrote:
There are two flags attemting to guard connector polling:
poll_enabled and poll_running. While poll_enabled semantics is clearly
defined and fully adhered (mark that drm_kms_helper_poll_init() was
called and not finalized by the _fini() call), the poll_running flag
doesn't have such clearliness.
This flag is used only in drm_helper_probe_single_connector_modes() to
guard calling of drm_kms_helper_poll_enable, it doesn't guard the
drm_kms_helper_poll_fini(), etc. Change it to only be set if the polling
is actually running. Tie HPD enablement to this flag.
This fixes the following warning reported after merging the HPD series:
Hot plug detection already enabled
WARNING: CPU: 2 PID: 9 at drivers/gpu/drm/drm_bridge.c:1257
drm_bridge_hpd_enable+0x94/0x9c [drm]
Modules linked in: videobuf2_memops snd_soc_simple_card
snd_soc_simple_card_utils fsl_imx8_ddr_perf videobuf2_common snd_soc_imx_spdif
adv7511 etnaviv imx8m_ddrc imx_dcss mc cec nwl_dsi gov
CPU: 2 PID: 9 Comm: kworker/u8:0 Not tainted 6.2.0-rc2-15208-g25b283acd578 #6
Hardware name: NXP i.MX8MQ EVK (DT)
Workqueue: events_unbound deferred_probe_work_func
pstate: 6005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : drm_bridge_hpd_enable+0x94/0x9c [drm]
lr : drm_bridge_hpd_enable+0x94/0x9c [drm]
sp : 89ef3740
x29: 89ef3740 x28: 09331f00 x27: 1000
x26: 0020 x25: 81148ed8 x24: 0a8fe000
x23: fffd x22: 05086348 x21: 81133ee0
x20: 0550d800 x19: 05086288 x18: 0006
x17: x16: 896ef008 x15: 972891004260
x14: 2a1403e19400 x13: 972891004260 x12: 2a1403e19400
x11: 7100385f29400801 x10: 0aa0 x9 : 88112744
x8 : 00250b00 x7 : 0003 x6 : 0011
x5 : x4 : bd986a48 x3 : 0001
x2 : x1 : x0 : 0025
Call trace:
drm_bridge_hpd_enable+0x94/0x9c [drm]
drm_bridge_connector_enable_hpd+0x2c/0x3c [drm_kms_helper]
drm_kms_helper_poll_enable+0x94/0x10c [drm_kms_helper]
drm_helper_probe_single_connector_modes+0x1a8/0x510 [drm_kms_helper]
drm_client_modeset_probe+0x204/0x1190 [drm]
__drm_fb_helper_initial_config_and_unlock+0x5c/0x4a4 [drm_kms_helper]
drm_fb_helper_initial_config+0x54/0x6c [drm_kms_helper]
drm_fbdev_client_hotplug+0xd0/0x140 [drm_kms_helper]
drm_fbdev_generic_setup+0x90/0x154 [drm_kms_helper]
dcss_kms_attach+0x1c8/0x254 [imx_dcss]
dcss_drv_platform_probe+0x90/0xfc [imx_dcss]
platform_probe+0x70/0xcc
really_probe+0xc4/0x2e0
__driver_probe_device+0x80/0xf0
driver_probe_device+0xe0/0x164
__device_attach_driver+0xc0/0x13c
bus_for_each_drv+0x84/0xe0
__device_attach+0xa4/0x1a0
device_initial_probe+0x1c/0x30
bus_probe_device+0xa4/0xb0
deferred_probe_work_func+0x90/0xd0
process_one_work+0x200/0x474
worker_thread+0x74/0x43c
kthread+0xfc/0x110
ret_from_fork+0x10/0x20
---[ end trace ]---
Reported-by: Laurentiu Palcu
Fixes: c8268795c9a9 ("drm/probe-helper: enable and disable HPD on connectors")
Tested-by: Marek Szyprowski
Tested-by: Chen-Yu Tsai
Acked-by: Laurentiu Palcu
Tested-by: Laurentiu Palcu
Tested-by: Laurent Pinchart
Signed-off-by: Dmitry Baryshkov
Thanks for your patch!
This gets rids of the warning splats on e.g. Renesas Koelsch and
Salvator-XS, so
Tested-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Intel-gfx] [PATCH v4] arch: rename all internal names __xchg to __arch_xchg
On Thu, Jan 5, 2023 at 10:54 AM Andrzej Hajda wrote:
> __xchg will be used for non-atomic xchg macro.
>
> Signed-off-by: Andrzej Hajda
> Reviewed-by: Arnd Bergmann
> ---
> v2: squashed all arch patches into one
> v3: fixed alpha/xchg_local, thx to l...@intel.com
> v4: adjusted indentation (Heiko)
> arch/m68k/include/asm/cmpxchg.h | 6 +++---
Acked-by: Geert Uytterhoeven [m68k]
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Intel-gfx] [PATCH v2] arch: rename all internal names __xchg to __arch_xchg
On Thu, Dec 29, 2022 at 12:34 PM Andrzej Hajda wrote:
> __xchg will be used for non-atomic xchg macro.
>
> Signed-off-by: Andrzej Hajda
> Reviewed-by: Arnd Bergmann
Acked-by: Geert Uytterhoeven [m68k]
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Intel-gfx] [PATCH 00/19] Introduce __xchg, non-atomic xchg
Hi Andrzej,
Thanks for your series!
On Thu, Dec 22, 2022 at 12:49 PM Andrzej Hajda wrote:
> I hope there will be place for such tiny helper in kernel.
> Quick cocci analyze shows there is probably few thousands places
> where it could be useful.
> I am not sure who is good person to review/ack such patches,
> so I've used my intuition to construct to/cc lists, sorry for mistakes.
> This is the 2nd approach of the same idea, with comments addressed[0].
>
> The helper is tiny and there are advices we can leave without it, so
> I want to present few arguments why it would be good to have it:
>
> 1. Code readability/simplification/number of lines:
>
> Real example from drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c:
> - previous_min_rate = evport->qos.min_rate;
> - evport->qos.min_rate = min_rate;
> + previous_min_rate = __xchg(evport->qos.min_rate, min_rate);
Upon closer look, shouldn't that be
previous_min_rate = __xchg(>qos.min_rate, min_rate);
?
> For sure the code is more compact, and IMHO more readable.
>
> 2. Presence of similar helpers in other somehow related languages/libs:
>
> a) Rust[1]: 'replace' from std::mem module, there is also 'take'
> helper (__xchg(, 0)), which is the same as private helper in
> i915 - fetch_and_zero, see latest patch.
> b) C++ [2]: 'exchange' from utility header.
>
> If the idea is OK there are still 2 qestions to answer:
>
> 1. Name of the helper, __xchg follows kernel conventions,
> but for me Rust names are also OK.
Before I realized the missing "&", I wondered how this is different
from swap(), so naming is important.
https://elixir.bootlin.com/linux/latest/source/include/linux/minmax.h#L139
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Intel-gfx] [Nouveau] [PATCH v7 00/23] drm: Analog TV Improvements
Hi Lukas,
On Tue, Nov 8, 2022 at 2:20 PM Lukas Satin wrote:
> One can switch from NTSC to PAL now using (on vc4)
>
> modetest -M vc4 -s 53:720x480i -w 53:'TV mode':1 # NTSC
> modetest -M vc4 -s 53:720x576i -w 53:'TV mode':4 # PAL
>
> NTSC should be 640x480i, not 720. It will probably work on most TV's, but
> NTSC by the spec is 640x480i.
The above are actually the digital ("DVD Video") variants, which have 720
horizontal pixels (incl. overscan).
The analog variants do not have a fixed horizontal resolution, except
for bandwidth limitations.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Intel-gfx] [PATCH v2 10/33] drm/modes: Add a function to generate analog display modes
Hi Maxime,
On Mon, Sep 26, 2022 at 12:17 PM Maxime Ripard wrote:
> On Fri, Sep 23, 2022 at 11:05:48AM +0200, Thomas Zimmermann wrote:
> > > + /* 63.556us * 13.5MHz = 858 pixels */
> >
> > I kind of get what the comment wants to tell me, but the units don't add up.
>
> I'm not sure how it doesn't add up?
>
> We have a frequency in Hz (equivalent to s^-1) and a duration in s, so
> the result ends up with no dimension, which is to be expected for a
> number of periods?
To make the units add up, it should be 13.5 Mpixel/s
(which is what a pixel clock of 13.5 MHz really means ;-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Intel-gfx] [PATCH v2 09/41] drm/connector: Add TV standard property
Hi Mateusz,
On Fri, Sep 2, 2022 at 12:00 AM Mateusz Kwiatkowski wrote:
> W dniu 29.08.2022 o 15:11, Maxime Ripard pisze:
> > The TV mode property has been around for a while now to select and get the
> > current TV mode output on an analog TV connector.
> >
> > Despite that property name being generic, its content isn't and has been
> > driver-specific which makes it hard to build any generic behaviour on top
> > of it, both in kernel and user-space.
> >
> > Let's create a new bitmask tv norm property, that can contain any of the
> > analog TV standards currently supported by kernel drivers. Each driver can
> > then pass in a bitmask of the modes it supports.
>
> This is not a bitmask property anymore, you've just changed it to an enum.
> The commit message is now misleading.
>
> > +static const struct drm_prop_enum_list drm_tv_mode_enum_list[] = {
> > +{ DRM_MODE_TV_MODE_NTSC_443, "NTSC-443" },
> > +{ DRM_MODE_TV_MODE_NTSC_J, "NTSC-J" },
> > +{ DRM_MODE_TV_MODE_NTSC_M, "NTSC-M" },
> > +{ DRM_MODE_TV_MODE_PAL_60, "PAL-60" },
> > +{ DRM_MODE_TV_MODE_PAL_B, "PAL-B" },
> > +{ DRM_MODE_TV_MODE_PAL_D, "PAL-D" },
> > +{ DRM_MODE_TV_MODE_PAL_G, "PAL-G" },
> > +{ DRM_MODE_TV_MODE_PAL_H, "PAL-H" },
> > +{ DRM_MODE_TV_MODE_PAL_I, "PAL-I" },
> > +{ DRM_MODE_TV_MODE_PAL_M, "PAL-M" },
> > +{ DRM_MODE_TV_MODE_PAL_N, "PAL-N" },
> > +{ DRM_MODE_TV_MODE_PAL_NC, "PAL-Nc" },
> > +{ DRM_MODE_TV_MODE_SECAM_60, "SECAM-60" },
> > +{ DRM_MODE_TV_MODE_SECAM_B, "SECAM-B" },
> > +{ DRM_MODE_TV_MODE_SECAM_D, "SECAM-D" },
> > +{ DRM_MODE_TV_MODE_SECAM_G, "SECAM-G" },
> > +{ DRM_MODE_TV_MODE_SECAM_K, "SECAM-K" },
> > +{ DRM_MODE_TV_MODE_SECAM_K1, "SECAM-K1" },
> > +{ DRM_MODE_TV_MODE_SECAM_L, "SECAM-L" },
> > +};
>
> I did not comment on it the last time, but this list looks a little bit
> random.
>
> Compared to the standards defined by V4L2, you also define SECAM-60 (a good
> thing to define, because why not), but don't define PAL-B1, PAL-D1, PAL-K,
> SECAM-H, SECAM-LC (whatever that is - probably just another name for SECAM-L,
> see my comment about PAL-Nc below), or NTSC-M-KR (a Korean variant of NTSC).
>
> Like I mentioned previously, I'm personally not a fan of including all those
> CCIR/ITU system variants, as they don't mean any difference to the output
> unless
> there is an RF modulator involved. But I get it that they have already been
> used
> and regressing probably wouldn't be a very good idea. But in that case keeping
> it consistent with the set of values used by V4L2 would be wise, I think.
Exactly. Anything outputting RGB (e.g. through a SCART or VGA connector)
doesn't care about the color subcarrier or modulator parts. Likewise,
anything outputting CVBS doesn't care about the modulator part.
Perhaps "generic" variants of NSTC and PAL/SECAM should be added, which
would really just mean 525/60 resp. 625/50.
Alternatively, the tv_mode field could be split in two parts (either
two separate fields, or bitwise), to maintain a clear separation between
lines/fields versus color encoding and RF modulation (with zero for the
latter meaning a generic version)? That would also keep the door open
for TV_MODE_405_50, TV_MODE_819_50, TV_MODE_750_50, TV_MODE_750_60, ...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Intel-gfx] [PATCH v2 10/41] drm/modes: Add a function to generate analog display modes
Hi Mateusz,
On Wed, Aug 31, 2022 at 3:44 AM Mateusz Kwiatkowski wrote:
> Wow. That's an enormous amount of effort put into this patch.
>
> But I'm tempted to say that this is actually overengineered quite a bit :D
> Considering that there's no way to access all these calculations from user
> space, and I can't imagine anybody using anything else than those standard
> 480i/576i (and maybe 240p/288p) modes at 13.5 MHz any time soon... I'm not
> sure if we actually need all this.
We'll need it when we get an Amiga DRM driver, which will use
7/14/28 MHz pixel clocks.
> But anyway, I'm not the maintainer of this subsystem, so I'm not the one to
> decide.
>
> > +enum drm_mode_analog {
> > +DRM_MODE_ANALOG_NTSC,
> > +DRM_MODE_ANALOG_PAL,
> > +};
>
> Using "NTSC" and "PAL" to describe the 50Hz and 60Hz analog TV modes is
> common,
> but strictly speaking a misnomer. Those are color encoding systems, and your
> patchset fully supports lesser used, but standard encodings for those (e.g.
> PAL-M for 60Hz and SECAM for 50Hz). I'd propose switching to some more neutral
> naming scheme. Some ideas:
>
> - DRM_MODE_ANALOG_60_HZ / DRM_MODE_ANALOG_50_HZ (after standard refresh rate)
> - DRM_MODE_ANALOG_525_LINES / DRM_MODE_ANALOG_625_LINES (after standard line
> count)
IMHO these are bad names, as e.g. VGA640x480@60 is also analog, using
60 Hz and 525 lines. Add "TV" to the name?
> - DRM_MODE_ANALOG_JM / DRM_MODE_ANALOG_BDGHIKLN (after corresponding ITU
> System
> Letter Designations)
Or DRM_MODE_ITU_*?
But given the long list of letters, this looks fragile to me.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Intel-gfx] [PATCH v2 14/41] drm/modes: Move named modes parsing to a separate function
Hi Maxime,
On Mon, Aug 29, 2022 at 3:13 PM Maxime Ripard wrote:
> The current construction of the named mode parsing doesn't allow to extend
> it easily. Let's move it to a separate function so we can add more
> parameters and modes.
>
> Signed-off-by: Maxime Ripard
Thanks for your patch!
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1909,6 +1909,9 @@ void drm_connector_list_update(struct drm_connector
> *connector)
> }
> EXPORT_SYMBOL(drm_connector_list_update);
>
> +#define STR_STRICT_EQ(str, len, cmp) \
> + ((strlen(cmp) == len) && !strncmp(str, cmp, len))
This is not part of the move, but newly added.
> +
> static int drm_mode_parse_cmdline_bpp(const char *str, char **end_ptr,
> struct drm_cmdline_mode *mode)
> {
> @@ -2208,6 +2211,52 @@ static const char * const drm_named_modes_whitelist[]
> = {
> "PAL",
> };
>
> +static int drm_mode_parse_cmdline_named_mode(const char *name,
> +unsigned int name_end,
> +struct drm_cmdline_mode
> *cmdline_mode)
> +{
> + unsigned int i;
> +
> + if (!name_end)
> + return 0;
This is already checked by the caller.
> +
> + /* If the name starts with a digit, it's not a named mode */
> + if (isdigit(name[0]))
> + return 0;
> +
> + /*
> +* If there's an equal sign in the name, the command-line
> +* contains only an option and no mode.
> +*/
> + if (strnchr(name, name_end, '='))
> + return 0;
> +
> + /* The connection status extras can be set without a mode. */
> + if (STR_STRICT_EQ(name, name_end, "d") ||
> + STR_STRICT_EQ(name, name_end, "D") ||
> + STR_STRICT_EQ(name, name_end, "e"))
> + return 0;
These checks are not part of the move, and should probably be added
in a separate patch.
> +
> + /*
> +* We're sure we're a named mode at that point, iterate over the
> +* list of modes we're aware of.
> +*/
> + for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++) {
> + int ret;
> +
> + ret = str_has_prefix(name, drm_named_modes_whitelist[i]);
> + if (ret != name_end)
> + continue;
> +
> + strcpy(cmdline_mode->name, drm_named_modes_whitelist[i]);
> + cmdline_mode->specified = true;
> +
> + return 1;
> + }
> +
> + return -EINVAL;
> +}
> +
> /**
> * drm_mode_parse_command_line_for_connector - parse command line modeline
> for connector
> * @mode_option: optional per connector mode option
> @@ -2244,7 +2293,7 @@ bool drm_mode_parse_command_line_for_connector(const
> char *mode_option,
> const char *bpp_ptr = NULL, *refresh_ptr = NULL, *extra_ptr = NULL;
> const char *options_ptr = NULL;
> char *bpp_end_ptr = NULL, *refresh_end_ptr = NULL;
> - int i, len, ret;
> + int len, ret;
>
> memset(mode, 0, sizeof(*mode));
> mode->panel_orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
> @@ -2285,17 +2334,19 @@ bool drm_mode_parse_command_line_for_connector(const
> char *mode_option,
> parse_extras = true;
> }
>
> - /* First check for a named mode */
> - for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++) {
> - ret = str_has_prefix(name, drm_named_modes_whitelist[i]);
> - if (ret == mode_end) {
> - if (refresh_ptr)
> - return false; /* named + refresh is invalid */
>
> - strcpy(mode->name, drm_named_modes_whitelist[i]);
> - mode->specified = true;
> - break;
> - }
> + if (mode_end) {
> + ret = drm_mode_parse_cmdline_named_mode(name, mode_end, mode);
> + if (ret < 0)
> + return false;
> +
> + /*
> +* Having a mode that starts by a letter (and thus is named)
> +* and an at-sign (used to specify a refresh rate) is
> +* disallowed.
> +*/
> + if (ret && refresh_ptr)
> + return false;
> }
>
> /* No named mode? Check for a normal mode argument, e.g. 1024x768 */
>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Intel-gfx] linux-next: manual merge of the drm tree with the drm-misc-fixes tree
Hi Stephen,
On Mon, Jul 18, 2022 at 1:49 AM Stephen Rothwell wrote:
> On Mon, 11 Jul 2022 10:05:45 +0200 Christian König
> wrote:
> > Am 11.07.22 um 04:47 schrieb Stephen Rothwell:
> > >
> > > Today's linux-next merge of the drm tree got a conflict in:
> > >
> > >drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> > >
> > > between commit:
> > >
> > >925b6e59138c ("Revert "drm/amdgpu: add drm buddy support to amdgpu"")
> > >
> > > from the drm-misc-fixes tree and commit:
> > >
> > >5e3f1e7729ec ("drm/amdgpu: fix start calculation in
> > > amdgpu_vram_mgr_new")
> > >
> > > from the drm tree.
> > >
> > > This is a mess :-( I have just reverted the above revert before mergin
> > > the drm tree for today, please fix it up.
> >
> > Sorry for the noise, the patch "5e3f1e7729ec ("drm/amdgpu: fix start
> > calculation in amdgpu_vram_mgr_new")" and another one is going to be
> > reverted from the drm tree as well.
> >
> > It's just that -fixes patches where faster than -next patches.
>
> Here we are a week later, -rc7 has been released and as far as I can
> tell (though I may have missed it), this is still a problem :-(
>
> I am still reverting 925b6e59138c (which is now in Linus' tree).
Thanks for the hint! After reverting that commit, drm-next (sort of[1])
merges cleanly into upstream again.
[1] There's still a small conflict due to the removal of
force_dpms_off, cfr. the difference between commits
3283c83eb6fcfbda and cc79950bf0904f58 ("drm/amd/display:
Ensure valid event timestamp for cursor-only commits") in
v5.19-rc7 resp. drm-next.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[Intel-gfx] [PATCH] drm/fb-helper: Remove helpers to change frame buffer config
The DRM fbdev emulation layer does not support pushing back
changes to fb_var_screeninfo to KMS.
However, drm_fb_helper still implements the fb_ops.fb_check_var() and
fb_ops.fb_set_par() callbacks, but the former fails to validate various
parameters passed from userspace. Hence unsanitized and possible
invaled values are passed up through the fbdev, fbcon, and console
stack, which has become an endless source of security issues reported
against fbdev.
Fix this by not populating the fb_ops.fb_check_var() and
fb_ops.fb_set_par() callbacks, as there is no point in providing them if
the frame buffer config cannot be changed anyway. This makes the fbdev
core aware that making changes to the frame buffer config is not
supported, so it will always return the current config.
Signed-off-by: Geert Uytterhoeven
---
The only remaining DRM driver that implements fb_ops.fb_check_var() is
also broken, as it fails to validate various parameters passed from
userspace. So vmw_fb_check_var() should either be fixed, or removed.
---
drivers/gpu/drm/drm_fb_helper.c| 180 ++---
drivers/gpu/drm/i915/display/intel_fbdev.c | 15 --
drivers/gpu/drm/omapdrm/omap_fbdev.c | 2 -
include/drm/drm_fb_helper.h| 16 --
4 files changed, 13 insertions(+), 200 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 2d4cee6a10e7..1041a11c410d7967 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -228,9 +228,18 @@ int drm_fb_helper_debug_leave(struct fb_info *info)
}
EXPORT_SYMBOL(drm_fb_helper_debug_leave);
-static int
-__drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper,
- bool force)
+/**
+ * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration
+ * @fb_helper: driver-allocated fbdev helper, can be NULL
+ *
+ * This should be called from driver's drm _driver.lastclose callback
+ * when implementing an fbcon on top of kms using this helper. This ensures
that
+ * the user isn't greeted with a black screen when e.g. X dies.
+ *
+ * RETURNS:
+ * Zero if everything went ok, negative error code otherwise.
+ */
+int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
{
bool do_delayed;
int ret;
@@ -242,16 +251,7 @@ __drm_fb_helper_restore_fbdev_mode_unlocked(struct
drm_fb_helper *fb_helper,
return 0;
mutex_lock(_helper->lock);
- if (force) {
- /*
-* Yes this is the _locked version which expects the master lock
-* to be held. But for forced restores we're intentionally
-* racing here, see drm_fb_helper_set_par().
-*/
- ret = drm_client_modeset_commit_locked(_helper->client);
- } else {
- ret = drm_client_modeset_commit(_helper->client);
- }
+ ret = drm_client_modeset_commit(_helper->client);
do_delayed = fb_helper->delayed_hotplug;
if (do_delayed)
@@ -263,22 +263,6 @@ __drm_fb_helper_restore_fbdev_mode_unlocked(struct
drm_fb_helper *fb_helper,
return ret;
}
-
-/**
- * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration
- * @fb_helper: driver-allocated fbdev helper, can be NULL
- *
- * This should be called from driver's drm _driver.lastclose callback
- * when implementing an fbcon on top of kms using this helper. This ensures
that
- * the user isn't greeted with a black screen when e.g. X dies.
- *
- * RETURNS:
- * Zero if everything went ok, negative error code otherwise.
- */
-int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
-{
- return __drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper, false);
-}
EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode_unlocked);
#ifdef CONFIG_MAGIC_SYSRQ
@@ -1254,25 +1238,6 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned
int cmd,
}
EXPORT_SYMBOL(drm_fb_helper_ioctl);
-static bool drm_fb_pixel_format_equal(const struct fb_var_screeninfo *var_1,
- const struct fb_var_screeninfo *var_2)
-{
- return var_1->bits_per_pixel == var_2->bits_per_pixel &&
- var_1->grayscale == var_2->grayscale &&
- var_1->red.offset == var_2->red.offset &&
- var_1->red.length == var_2->red.length &&
- var_1->red.msb_right == var_2->red.msb_right &&
- var_1->green.offset == var_2->green.offset &&
- var_1->green.length == var_2->green.length &&
- var_1->green.msb_right == var_2->green.msb_right &&
- var_1->blue.offset == var_2->blue.offset &&
- var_1->blue.length == var_2->blue.length &&
- va
Re: [Intel-gfx] [PATCH][next] treewide: uapi: Replace zero-length arrays with flexible-array members
Hi Gustavo,
Thanks for your patch!
On Mon, Jun 27, 2022 at 8:04 PM Gustavo A. R. Silva
wrote:
> There is a regular need in the kernel to provide a way to declare
> having a dynamically sized set of trailing elements in a structure.
> Kernel code should always use “flexible array members”[1] for these
> cases. The older style of one-element or zero-length arrays should
> no longer be used[2].
These rules apply to the kernel, but uapi is not considered part of the
kernel, so different rules apply. Uapi header files should work with
whatever compiler that can be used for compiling userspace.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Intel-gfx] [PATCH v2 18/19] Revert "fbdev: Prevent probing generic drivers if a FB is already registered"
probably need to move that into fbmem.c and it all gets a bit messy.
>
> > Let me know if you think that makes sense and I can attempt to write a fix.
>
> I still think unregistering the platform_dev properly makes the most
That doesn't sound very driver-model-aware to me. The device is what
the driver binds to; it does not cease to exist.
> sense, and feels like the most proper linux device model solution
> instead of hacks on top - if the firmware fb is unuseable because a
> native driver has taken over, we should nuke that. And also the
> firmware fb driver would then just bind to that platform_dev if it
> exists, and only if it exists. Also I think it should be the
> responsibility of whichever piece of code that registers these
> platform devices to ensure that platform_dev actually still exists.
> That's why I think pushing all that code into sysfb.c is probably the
> cleanest solution.
Can't you unbind the generic driver first, and bind the specific driver
afterwards? Alike writing to sysfs unbind/driver_override/bind,
but from code?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Intel-gfx] [PATCH v1 1/1] drm/i915/selftests: Replace too verbose for-loop with simpler while
On Wed, Feb 16, 2022 at 9:55 AM Jani Nikula wrote:
> On Tue, 15 Feb 2022, Andy Shevchenko
> wrote:
> > On Tue, Feb 15, 2022 at 07:14:49PM +0200, Jani Nikula wrote:
> >> On Tue, 15 Feb 2022, Andy Shevchenko
> >> wrote:
> >> > It's hard to parse for-loop which has some magic calculations inside.
> >> > Much cleaner to use while-loop directly.
> >>
> >> I assume you're trying to prove a point following our recent
> >> for-vs-while loop discussion. I really can't think of any other reason
> >> you'd end up looking at this file or this loop.
> >>
> >> With the change, the loop indeed becomes simpler, but it also runs one
> >> iteration further than the original. Whoops.
> >
> > Yeah, sorry for that, the initial condition should be d = depth - 1,
> > of course.
>
> Well, no, the condition should be while (--i) instead to also match the
> values the original loop takes. ;D
"There are two hard things in computer science: cache invalidation,
naming things, and off-by-one errors."
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Intel-gfx] [PATCH 04/21] fbcon: delete a few unneeded forward decl
On Tue, Feb 1, 2022 at 9:50 PM Daniel Vetter wrote:
> I didn't bother with any code movement to fix the others, these just
> got a bit in the way.
>
> Signed-off-by: Daniel Vetter
Reviewed-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Intel-gfx] [PATCH 21/21] fbdev: Make registered_fb[] private to fbmem.c
Hi Daniel,
Thanks for your patch!
On Tue, Feb 1, 2022 at 9:50 PM Daniel Vetter wrote:
> Well except when the olpc dcon fbdev driver is enabled, that thing
> digs around in there in rather unfixable ways.
Can't the actual frame buffer driver (which one?) used on olpc export
a pointer to its fb_info?
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -48,10 +48,14 @@
> static DEFINE_MUTEX(registration_lock);
>
> struct fb_info *registered_fb[FB_MAX] __read_mostly;
> -EXPORT_SYMBOL(registered_fb);
> -
> int num_registered_fb __read_mostly;
> +#if IS_ENABLED(CONFIG_OLPC_DCON)
CONFIG_FB_OLPC_DCON (everywhere), cfr. the build failure reported
by the robot.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Intel-gfx] [PATCH 09/21] fbcon: Replace FBCON_FLAGS_INIT with a boolean
On Wed, Feb 2, 2022 at 10:25 AM Thomas Zimmermann wrote:
> Am 31.01.22 um 22:05 schrieb Daniel Vetter:
> > It's only one flag and slightly tidier code.
> >
> > Signed-off-by: Daniel Vetter
> > Cc: Daniel Vetter
> > Cc: Tetsuo Handa
> > Cc: Greg Kroah-Hartman
> > Cc: Du Cheng
> > Cc: Thomas Zimmermann
> > Cc: Claudio Suarez
>
> Acked-by: Thomas Zimmermann
> > +++ b/drivers/video/fbdev/core/fbcon.h
> > @@ -18,8 +18,6 @@
> >
> > #include
> >
> > -#define FBCON_FLAGS_INIT 1
> > -
> > /*
> > *This is the interface between the low-level console driver and
> > the
> > *low-level frame buffer device
> > @@ -77,7 +75,7 @@ struct fbcon_ops {
> > intblank_state;
> > intgraphics;
> > intsave_graphics; /* for debug enter/leave */
> > - intflags;
> > + bool initialized;
>
> This will add 3 bytes of padding. Maybe you can put the bool elsewhere.
Several of the int variables are used as boolean flags, too.
Perhaps convert them all to bitfields?
unsigned int initialized : 1;
...
> > introtate;
> > intcur_rotate;
> > char *cursor_data;
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Intel-gfx] [PATCH 01/21] MAINTAINERS: Add entry for fbdev core
On Mon, Jan 31, 2022 at 10:06 PM Daniel Vetter wrote:
> Ever since Tomi extracted the core code in 2014 it's been defacto me
> maintaining this, with help from others from dri-devel and sometimes
> Linus (but those are mostly merge conflicts):
>
> $ git shortlog -ns drivers/video/fbdev/core/ | head -n5
> 35 Daniel Vetter
> 23 Linus Torvalds
> 10 Hans de Goede
> 9 Dave Airlie
> 6 Peter Rosin
>
> I think ideally we'd also record that the various firmware fb drivers
> (efifb, vesafb, ...) are also maintained in drm-misc because for the
> past few years the patches have either been to fix handover issues
> with drm drivers, or caused handover issues with drm drivers. So any
> other tree just doesn't make sense. But also, there's plenty of
> outdated MAINTAINER entries for these with people and git trees that
> haven't been active in years, so maybe let's just leave them alone.
> And furthermore distros are now adopting simpledrm as the firmware fb
> driver, so hopefully the need to care about the fbdev firmware drivers
> will go down going forward.
>
> Note that drm-misc is group maintained, I expect that to continue like
> we've done before, so no new expectations that patches all go through
> my hands. That would be silly. This also means I'm happy to put any
> other volunteer's name in the M: line, but otherwise git log says I'm
> the one who's stuck with this.
> Signed-off-by: Daniel Vetter
Acked-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Intel-gfx] [PATCH v0 42/42] notifier: Return an error when callback is already registered
Hi Borislav,
On Mon, Nov 8, 2021 at 4:59 PM Borislav Petkov wrote:
> On Mon, Nov 08, 2021 at 04:25:47PM +0100, Geert Uytterhoeven wrote:
> > I'm not against returning proper errors codes. I'm against forcing
> > callers to check things that cannot fail and to add individual error
> > printing to each and every caller.
>
> If you're against checking things at the callers, then the registration
> function should be void. IOW, those APIs are not optimally designed atm.
Returning void is the other extreme ;-)
There are 3 levels (ignoring BUG_ON()/panic () inside the callee):
1. Return void: no one can check success or failure,
2. Return an error code: up to the caller to decide,
3. Return a __must_check error code: every caller must check.
I'm in favor of 2, as there are several places where it cannot fail.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Intel-gfx] [PATCH v0 42/42] notifier: Return an error when callback is already registered
Hi Borislav,
On Mon, Nov 8, 2021 at 3:21 PM Borislav Petkov wrote:
> On Mon, Nov 08, 2021 at 03:07:03PM +0100, Geert Uytterhoeven wrote:
> > I think the addition of __must_check is overkill, leading to the
> > addition of useless error checks and message printing.
>
> See the WARN in notifier_chain_register() - it will already do "message
> printing".
I mean the addition of useless error checks and message printing _to
the callers_.
> > Many callers call this where it cannot fail, and where nothing can
> > be done in the very unlikely event that the call would ever start to
> > fail.
>
> This is an attempt to remove this WARN() hack in
> notifier_chain_register() and have the function return a proper error
> value instead of this "Currently always returns zero." which is bad
> design.
>
> Some of the registration functions around the tree check that retval and
> some don't. So if "it cannot fail" those registration either should not
> return a value or callers should check that return value - what we have
> now doesn't make a whole lot of sense.
With __must_check callers are required to check, even if they know
it cannot fail.
> Oh, and then fixing this should avoid stuff like:
>
> + if (notifier_registered == false) {
> + mce_register_decode_chain(_bad_page_nb);
> + notifier_registered = true;
> + }
>
> from propagating in the code.
That's unrelated to the addition of __must_check.
I'm not against returning proper errors codes. I'm against forcing
callers to check things that cannot fail and to add individual error
printing to each and every caller.
Note that in other areas, we are moving in the other
direction, to a centralized printing of error messages,
cfr. e.g. commit 7723f4c5ecdb8d83 ("driver core: platform: Add an
error message to platform_get_irq*()").
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Intel-gfx] [PATCH v0 42/42] notifier: Return an error when callback is already registered
Hi Borislav,
On Mon, Nov 8, 2021 at 11:13 AM Borislav Petkov wrote:
> From: Borislav Petkov
>
> The notifier registration routine doesn't return a proper error value
> when a callback has already been registered, leading people to track
> whether that registration has happened at the call site:
>
> https://lore.kernel.org/amd-gfx/20210512013058.6827-1-mukul.jo...@amd.com/
>
> Which is unnecessary.
>
> Return -EEXIST to signal that case so that callers can act accordingly.
> Enforce callers to check the return value, leading to loud screaming
> during build:
>
> arch/x86/kernel/cpu/mce/core.c: In function ‘mce_register_decode_chain’:
> arch/x86/kernel/cpu/mce/core.c:167:2: error: ignoring return value of \
>‘blocking_notifier_chain_register’, declared with attribute
> warn_unused_result [-Werror=unused-result]
> blocking_notifier_chain_register(_mce_decoder_chain, nb);
> ^~~~
>
> Drop the WARN too, while at it.
>
> Suggested-by: Thomas Gleixner
> Signed-off-by: Borislav Petkov
Thanks for your patch!
> --- a/include/linux/notifier.h
> +++ b/include/linux/notifier.h
> @@ -141,13 +141,13 @@ extern void srcu_init_notifier_head(struct
> srcu_notifier_head *nh);
>
> #ifdef __KERNEL__
>
> -extern int atomic_notifier_chain_register(struct atomic_notifier_head *nh,
> +extern int __must_check atomic_notifier_chain_register(struct
> atomic_notifier_head *nh,
> struct notifier_block *nb);
> -extern int blocking_notifier_chain_register(struct blocking_notifier_head
> *nh,
> +extern int __must_check blocking_notifier_chain_register(struct
> blocking_notifier_head *nh,
> struct notifier_block *nb);
> -extern int raw_notifier_chain_register(struct raw_notifier_head *nh,
> +extern int __must_check raw_notifier_chain_register(struct raw_notifier_head
> *nh,
> struct notifier_block *nb);
> -extern int srcu_notifier_chain_register(struct srcu_notifier_head *nh,
> +extern int __must_check srcu_notifier_chain_register(struct
> srcu_notifier_head *nh,
> struct notifier_block *nb);
I think the addition of __must_check is overkill, leading to the
addition of useless error checks and message printing. Many callers
call this where it cannot fail, and where nothing can be done in the
very unlikely event that the call would ever start to fail.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Intel-gfx] mmotm 2021-10-05-19-53 uploaded (drivers/gpu/drm/msm/hdmi/hdmi_phy.o)
Hi Christian,
On Wed, Oct 6, 2021 at 9:28 AM Christian König wrote:
> Am 06.10.21 um 09:20 schrieb Stephen Rothwell:
> > On Tue, 5 Oct 2021 22:48:03 -0700 Randy Dunlap
> > wrote:
> >> on i386:
> >>
> >> ld: drivers/gpu/drm/msm/hdmi/hdmi_phy.o:(.rodata+0x3f0): undefined
> >> reference to `msm_hdmi_phy_8996_cfg'
> >>
> >>
> >> Full randconfig fle is attached.
> > This would be because CONFIG_DRM_MSM is set but CONFIG_COMMON_CLOCK is
> > not and has been exposed by commit
> >
> >b3ed524f84f5 ("drm/msm: allow compile_test on !ARM")
> >
> > from the drm-misc tree.
>
> Good point, how about this change:
>
> diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
> index 5879f67bc88c..d9879b011fb0 100644
> --- a/drivers/gpu/drm/msm/Kconfig
> +++ b/drivers/gpu/drm/msm/Kconfig
> @@ -5,7 +5,7 @@ config DRM_MSM
> depends on DRM
> depends on ARCH_QCOM || SOC_IMX5 || COMPILE_TEST
> depends on IOMMU_SUPPORT
> - depends on (OF && COMMON_CLK) || COMPILE_TEST
> + depends on (OF || COMPILE_TEST) && COMMON_CLK
I'd make that:
-depends on DRM
+ depends on COMMON_CLK && DRM && IOMMU_SUPPORT
depends on ARCH_QCOM || SOC_IMX5 || COMPILE_TEST
-depends on IOMMU_SUPPORT
- depends on (OF && COMMON_CLK) || COMPILE_TEST
+ depends on OF || COMPILE_TEST
to keep a better separation between hard and soft dependencies.
Note that the "depends on OF || COMPILE_TEST" can even be
deleted, as the dependency on ARCH_QCOM || SOC_IMX5 implies OF.
> depends on QCOM_OCMEM || QCOM_OCMEM=n
> depends on QCOM_LLCC || QCOM_LLCC=n
> depends on QCOM_COMMAND_DB || QCOM_COMMAND_DB=n
>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Intel-gfx] [PATCH 000/141] Fix fall-through warnings for Clang
Hi Miguel,
On Thu, Nov 26, 2020 at 3:54 PM Miguel Ojeda
wrote:
> On Wed, Nov 25, 2020 at 11:44 PM Edward Cree wrote:
> > To make the intent clear, you have to first be certain that you
> > understand the intent; otherwise by adding either a break or a
> > fallthrough to suppress the warning you are just destroying the
> > information that "the intent of this code is unknown".
>
> If you don't know what the intent of your own code is, then you
> *already* have a problem in your hands.
The maintainer is not necessarily the owner/author of the code, and
thus may not know the intent of the code.
> > or does it flag up code
> > that can be mindlessly "fixed" (in which case the warning is
> > worthless)? Proponents in this thread seem to be trying to
> > have it both ways.
>
> A warning is not worthless just because you can mindlessly fix it.
> There are many counterexamples, e.g. many
> checkpatch/lint/lang-format/indentation warnings, functional ones like
> the `if (a = b)` warning...
BTW, you cannot mindlessly fix the latter, as you cannot know if
"(a == b)" or "((a = b))" was intended, without understanding the code
(and the (possibly unavailable) data sheet, and the hardware, ...).
P.S. So far I've stayed out of this thread, as I like it if the compiler
flags possible mistakes. After all I was the one fixing new
"may be used uninitialized" warnings thrown up by gcc-4.1, until
(a bit later than) support for that compiler was removed...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] fbcon: Disable accelerated scrolling
Hi Daniel,
Replying "early" (see below), as this was applied to
drm-misc/for-linux-next.
On Sat, Oct 31, 2020 at 3:17 PM Daniel Vetter wrote:
> On Sat, Oct 31, 2020 at 11:28 AM Geert Uytterhoeven
> wrote:
> > On Thu, 29 Oct 2020, Daniel Vetter wrote:
> > > So ever since syzbot discovered fbcon, we have solid proof that it's
> > > full of bugs. And often the solution is to just delete code and remove
> > > features, e.g. 50145474f6ef ("fbcon: remove soft scrollback code").
> > >
> > > Now the problem is that most modern-ish drivers really only treat
> > > fbcon as an dumb kernel console until userspace takes over, and Oops
> > > printer for some emergencies. Looking at drm drivers and the basic
> > > vesa/efi fbdev drivers shows that only 3 drivers support any kind of
> > > acceleration:
> > >
> > > - nouveau, seems to be enabled by default
> > > - omapdrm, when a DMM remapper exists using remapper rewriting for
> > > y/xpanning
> > > - gma500, but that is getting deleted now for the GTT remapper trick,
> > > and the accelerated copyarea never set the FBINFO_HWACCEL_COPYAREA
> > > flag, so unused (and could be deleted already I think).
> > >
> > > No other driver supportes accelerated fbcon. And fbcon is the only
> > > user of this accel code (it's not exposed as uapi through ioctls),
> > > which means we could garbage collect fairly enormous amounts of code
> > > if we kill this.
> >
> > "git grep FBINFO_HWACCEL_COPYAREA" shows me there are 32 more drivers
> > using acceleration under drivers/video/fbdev/.
> >
> > > Plus because syzbot only runs on virtual hardware, and none of the
> > > drivers for that have acceleration, we'd remove a huge gap in testing.
> > > And there's no other even remotely comprehensive testing aside from
> > > syzbot.
> >
> > That sounds like a great argument to remove all hardware drivers from
> > the kernel ;-)
>
> fbdev is unmaintained, has no one volunteering to put in the work (and
> there's huge amounts of work needed), and there's no test suite. No,
> fbtest.c doesn't can't, that's not even close. We're not going to
> delete everything in the kernel, but slowly sunsetting stuff that's
> just costing and not bringing in up is a good idea.
The fbcon acceleration code is indeed not tested by fbset, and it is
purely in-kernel acceleration for the console.
> > Seriously, how hard can it be to add "software-accelerated" acceleration
> > hooks to drivers/video/fbdev/vfb.c, to enable syzbot to exercise the
> > core acceleration code paths?
>
> Just this one is 5 combinations, which means I'd need to convince
> syzbot to test 5 different machine setups.
Why 5 combinations?
Enable vfb (which can be a module) and be done with it?
> Plus we're still lacking a test suite, and judging from how much time
> it took to get something basic going for kms, that's about 2 engineer
> years of effort that no one is even close to willing to spend.
Sure, writing test suites is hard, and takes time.
> > > This patch here just disables the acceleration code by always
> > > redrawing when scrolling. The plan is that once this has been merged
> > > for well over a year in released kernels, we can start to go around
> > > and delete a lot of code.
> >
> > Have you benchmarked the performance impact on traditional fbdev
> > drivers?
>
> There's still some acceleration if you have an image blit engine for
> redrawing the screen. But the complexity is contained in the old
> drivers that no one cares about.
>
> For anything I have access to the difference is 0.
Sure, you're doing DRM drivers ;-)
> Reality is that fbdev is just there nowadays for Oops printing and
> emergency usage, and it's plenty good enough for that. If there's
That's true for systems that are supported by a DRM driver.
> anyone who cares beyond that, they're most definitely not able to put
> in time for upstream work.
There exist actual products using out-of-tree fbdev drivers that never
got the chance of being merged upstream due to the moratorium on new
fbdev drivers.
BTW, I'm trying to convert an old fbdev driver to DRM, but don't have it
working yet. I had hoped to get something working before replying to
this email, so I could provide more detailed feedback.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] fbcon: Disable accelerated scrolling
p & FBINFO_HWACCEL_DISABLED))
- p->scrollmode = SCROLL_MOVE;
- else /* default to something safe */
- p->scrollmode = SCROLL_REDRAW;
+ /*
+* No more hw acceleration for fbcon.
+*
+* FIXME: Garbage collect all the now dead code after sufficient time
+* has passed.
+*/
+ p->scrollmode = SCROLL_REDRAW;
/*
* ++guenther: console.c:vc_allocate() relies on initializing
@@ -1961,45 +1962,15 @@ static void updatescrollmode(struct fbcon_display *p,
{
struct fbcon_ops *ops = info->fbcon_par;
int fh = vc->vc_font.height;
- int cap = info->flags;
- u16 t = 0;
- int ypan = FBCON_SWAP(ops->rotate, info->fix.ypanstep,
- info->fix.xpanstep);
- int ywrap = FBCON_SWAP(ops->rotate, info->fix.ywrapstep, t);
int yres = FBCON_SWAP(ops->rotate, info->var.yres, info->var.xres);
int vyres = FBCON_SWAP(ops->rotate, info->var.yres_virtual,
info->var.xres_virtual);
- int good_pan = (cap & FBINFO_HWACCEL_YPAN) &&
- divides(ypan, vc->vc_font.height) && vyres > yres;
- int good_wrap = (cap & FBINFO_HWACCEL_YWRAP) &&
- divides(ywrap, vc->vc_font.height) &&
- divides(vc->vc_font.height, vyres) &&
- divides(vc->vc_font.height, yres);
- int reading_fast = cap & FBINFO_READS_FAST;
- int fast_copyarea = (cap & FBINFO_HWACCEL_COPYAREA) &&
- !(cap & FBINFO_HWACCEL_DISABLED);
- int fast_imageblit = (cap & FBINFO_HWACCEL_IMAGEBLIT) &&
- !(cap & FBINFO_HWACCEL_DISABLED);
p->vrows = vyres/fh;
if (yres > (fh * (vc->vc_rows + 1)))
p->vrows -= (yres - (fh * vc->vc_rows)) / fh;
if ((yres % fh) && (vyres % fh < yres % fh))
p->vrows--;
-
- if (good_wrap || good_pan) {
- if (reading_fast || fast_copyarea)
- p->scrollmode = good_wrap ?
- SCROLL_WRAP_MOVE : SCROLL_PAN_MOVE;
- else
- p->scrollmode = good_wrap ? SCROLL_REDRAW :
- SCROLL_PAN_REDRAW;
- } else {
- if (reading_fast || (fast_copyarea && !fast_imageblit))
- p->scrollmode = SCROLL_MOVE;
- else
- p->scrollmode = SCROLL_REDRAW;
- }
}
#define PITCH(w) (((w) + 7) >> 3)
--
2.28.0
___
dri-devel mailing list
dri-de...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 37/52] drm/rcar-du: Drop explicit drm_mode_config_cleanup call
Hi Daniel,
On Wed, Feb 19, 2020 at 11:57 AM Daniel Vetter wrote:
> On Wed, Feb 19, 2020 at 11:30 AM Geert Uytterhoeven
> wrote:
> > On Wed, Feb 19, 2020 at 11:22 AM Daniel Vetter
> > wrote:
> > > It's right above the drm_dev_put().
> > >
> > > Aside: Another driver with a bit much devm_kzalloc, which should
> > > probably use drmm_kzalloc instead ...
> >
> > What's drmm_kzalloc()?
> > The only references I can find are in this patch series.
>
> Yup, it's all new. Read cover letter for reading instructions for the
> entire patch series. I'm afraid the driver patches wont make much
> sense without the context. None actually :-/
IC, as the cover letter was sent only to dri-devel and intel-gfx, many
recipients of the patches won't have received it...
https://lore.kernel.org/dri-devel/20200219102122.1607365-1-daniel.vet...@ffwll.ch/
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 37/52] drm/rcar-du: Drop explicit drm_mode_config_cleanup call
Hi Daniel,
On Wed, Feb 19, 2020 at 11:22 AM Daniel Vetter wrote:
> It's right above the drm_dev_put().
>
> Aside: Another driver with a bit much devm_kzalloc, which should
> probably use drmm_kzalloc instead ...
What's drmm_kzalloc()?
The only references I can find are in this patch series.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 19/24] drm/bridge: dumb-vga-dac: Provide ddc symlink in connector sysfs directory
Hi Günter,
On Thu, Aug 8, 2019 at 5:42 AM Guenter Roeck wrote:
> On Fri, Jul 26, 2019 at 07:23:13PM +0200, Andrzej Pietrasiewicz wrote:
> > Use the ddc pointer provided by the generic connector.
> >
> > Signed-off-by: Andrzej Pietrasiewicz
> > Reviewed-by: Neil Armstrong
>
> This patch results in a crash when running qemu:versatilepb.
>
> Unable to handle kernel NULL pointer dereference at virtual address 00c5
> pgd = (ptrval)
> [00c5] *pgd=
> Internal error: Oops: 5 [#1] ARM
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper Not tainted 5.3.0-rc1+ #1
> Hardware name: ARM-Versatile (Device Tree Support)
> PC is at sysfs_do_create_link_sd+0x38/0xd8
> LR is at sysfs_do_create_link_sd+0x38/0xd8
> [] (sysfs_do_create_link_sd) from []
> (drm_connector_register.part.1+0x40/0xa0)
> [] (drm_connector_register.part.1) from []
> (drm_connector_register_all+0x90/0xb8)
> [] (drm_connector_register_all) from []
> (drm_modeset_register_all+0x44/0x6c)
> [] (drm_modeset_register_all) from []
> (drm_dev_register+0x15c/0x1c0)
> [] (drm_dev_register) from []
> (pl111_amba_probe+0x2e0/0x4ac)
> [] (pl111_amba_probe) from [] (amba_probe+0x9c/0x118)
Seeing the same thing on Salvator-XS, due to vga->ddc being -ENODEV.
> # first bad commit: [a4f9087e85de141e4e6d21ac2c583ae096cc9aba] drm/bridge:
> dumb-vga-dac: Provide ddc symlink in connector sysfs directory
Fix sent
https://lore.kernel.org/lkml/20190813093046.4976-1-geert+rene...@glider.be/
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH trivial] drm/i915: Grammar s/the its/its/
Signed-off-by: Geert Uytterhoeven
---
drivers/gpu/drm/i915/intel_dpll_mgr.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h
b/drivers/gpu/drm/i915/intel_dpll_mgr.h
index 8835dd20f1d27e05..79a53c5439a8e6db 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
@@ -293,7 +293,7 @@ struct intel_shared_dpll {
/**
* @state:
*
-* Store the state for the pll, including the its hw state
+* Store the state for the pll, including its hw state
* and CRTCs using it.
*/
struct intel_shared_dpll_state state;
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 20/33] fbdev/sh_mob: Remove fb notifier callback
On Mon, May 20, 2019 at 10:22 AM Daniel Vetter wrote:
> This seems to be entirely defunct:
>
> - The FB_EVEN_SUSPEND/RESUME events are only sent out by
> fb_set_suspend. Which is supposed to be called by drivers in their
> suspend/resume hooks, and not itself call into drivers. Luckily
> sh_mob doesn't call fb_set_suspend, so this seems to do nothing
> useful.
>
> - The notify hook calls sh_mobile_fb_reconfig() which in turn can
> call into the fb notifier. Or attempt too, since that would
> deadlock.
>
> So looks like leftover hacks from when this was originally introduced
> in
>
> commit 6011bdeaa6089d49c02de69f05980da7bad314ab
> Author: Guennadi Liakhovetski
> Date: Wed Jul 21 10:13:21 2010 +
>
> fbdev: sh-mobile: HDMI support for SH-Mobile SoCs
>
> So let's just remove it.
>
> Signed-off-by: Daniel Vetter
Reviewed-by: Geert Uytterhoeven
Display still works fine on Armadillo800-EVA, before and after system
suspend/resume, so:
Tested-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/33] fbdev/sh_mobile: remove sh_mobile_lcdc_display_notify
On Mon, May 20, 2019 at 10:25 AM Daniel Vetter wrote:
> It's dead code, and removing it avoids me having to understand
> what it's doing with lock_fb_info.
>
> Signed-off-by: Daniel Vetter
Reviewed-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 3/7] lib/hexdump.c: Optionally suppress lines of repeated bytes
Hi Alastair,
Thanks for your patch!
On Wed, May 8, 2019 at 9:04 AM Alastair D'Silva wrote:
> From: Alastair D'Silva
>
> Some buffers may only be partially filled with useful data, while the rest
> is padded (typically with 0x00 or 0xff).
>
> This patch introduces a flag to allow the supression of lines of repeated
> bytes,
Given print_hex_dump() operates on entities of groupsize (1, 2, 4, or 8)
bytes, wouldn't it make more sense to consider repeated groups instead
of repeated bytes?
> which are replaced with '** Skipped %u bytes of value 0x%x **'
Using a custom message instead of just "*", like "hexdump" uses, will
require preprocessing the output when recovering the original binary
data by feeding it to e.g. "xxd".
This may sound worse than it is, though, as I never got "xxd" to work
without preprocessing anyway ;-)
$ cat $(type -p unhexdump)
#!/bin/sh
sed 's/^[0-9a-f]*//' $1 | xxd -r -p | dd conv=swab
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] linux-next: manual merge of the mfd tree with the drm-intel tree
On Thu, Aug 16, 2018 at 8:05 PM Stephen Rothwell wrote:
> On Thu, 26 Jul 2018 15:10:06 +1000 Stephen Rothwell
> wrote:
> > Today's linux-next merge of the mfd tree got a conflict in:
> >
> > drivers/gpu/drm/i915/intel_display.h
> >
> > between commit:
> >
> > 6075546f57f8 ("drm/i915/icl: store the port type for TC ports")
> >
> > from the drm-intel tree and commit:
> >
> > 9c229127aee2 ("drm/i915: hdmi: add CEC notifier to intel_hdmi")
> >
> > from the mfd tree.
> >
> > I fixed it up (see below) and can carry the fix as necessary. This
> > is now fixed as far as linux-next is concerned, but any non trivial
> > conflicts should be mentioned to your upstream maintainer when your tree
> > is submitted for merging. You may also want to consider cooperating
> > with the maintainer of the conflicting tree to minimise any particularly
> > complex conflicts.
> >
> > --
> > Cheers,
> > Stephen Rothwell
> >
> > diff --cc drivers/gpu/drm/i915/intel_display.h
> > index 0a79a46d5805,1f176a71e081..
> > --- a/drivers/gpu/drm/i915/intel_display.h
> > +++ b/drivers/gpu/drm/i915/intel_display.h
> > @@@ -126,24 -126,30 +126,48 @@@ enum port
> >
> > #define port_name(p) ((p) + 'A')
> >
> > + /*
> > + * Ports identifier referenced from other drivers.
> > + * Expected to remain stable over time
> > + */
> > + static inline const char *port_identifier(enum port port)
> > + {
> > + switch (port) {
> > + case PORT_A:
> > + return "Port A";
> > + case PORT_B:
> > + return "Port B";
> > + case PORT_C:
> > + return "Port C";
> > + case PORT_D:
> > + return "Port D";
> > + case PORT_E:
> > + return "Port E";
> > + case PORT_F:
> > + return "Port F";
> > + default:
> > + return "";
> > + }
> > + }
> > +
> > +enum tc_port {
> > +PORT_TC_NONE = -1,
> > +
> > +PORT_TC1 = 0,
> > +PORT_TC2,
> > +PORT_TC3,
> > +PORT_TC4,
> > +
> > +I915_MAX_TC_PORTS
> > +};
> > +
> > +enum tc_port_type {
> > +TC_PORT_UNKNOWN = 0,
> > +TC_PORT_TYPEC,
> > +TC_PORT_TBT,
> > +TC_PORT_LEGACY,
> > +};
> > +
> > enum dpio_channel {
> > DPIO_CH0,
> > DPIO_CH1
>
> This is now a conflict between Linus' tree and the mfd tree.
FTR, it's not this one, but the similar one reported on Jul 19, which does not
add the tc_port_type enum.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/12] nubus: use for_each_if
Hi Daniel,
On Mon, Jul 9, 2018 at 10:44 AM Daniel Vetter wrote:
> Avoids the inverted check compared to the open-coded version.
>
> Signed-off-by: Daniel Vetter
> Cc: Finn Thain
> Cc: linux-m...@lists.linux-m68k.org
Thanks for your patch!
> --- a/include/linux/nubus.h
> +++ b/include/linux/nubus.h
> @@ -127,7 +127,7 @@ struct nubus_rsrc *nubus_next_rsrc_or_null(struct
> nubus_rsrc *from);
> for (f = nubus_first_rsrc_or_null(); f; f =
> nubus_next_rsrc_or_null(f))
>
> #define for_each_board_func_rsrc(b, f) \
> - for_each_func_rsrc(f) if (f->board != b) {} else
> + for_each_func_rsrc(f) for_each_if (f->board == b)
drivers/net/ethernet/8390/mac8390.c: In function ‘mac8390_device_probe’:
drivers/net/ethernet/8390/mac8390.c:402: error: implicit declaration
of function ‘for_each_if’
drivers/net/ethernet/8390/mac8390.c:402: error: expected ‘;’ before ‘{’ token
Apparently this depends on patch [01/12], which wasn't CCed to
linux-m...@lists.linux-m68k.org?
Please don't split CCs if all patches in the a series are not independent.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Spelling s/auxilliary/auxiliary/
Signed-off-by: Geert Uytterhoeven geert+rene...@glider.be --- drivers/gpu/drm/i915/intel_runtime_pm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index 6d8e29abbc333c87..74fdd8ca2cb2a347 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -1420,7 +1420,7 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv) } /** - * intel_aux_display_runtime_get - grab an auxilliary power domain reference + * intel_aux_display_runtime_get - grab an auxiliary power domain reference * @dev_priv: i915 device instance * * This function grabs a power domain reference for the auxiliary power domain @@ -1437,10 +1437,10 @@ void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv) } /** - * intel_aux_display_runtime_put - release an auxilliary power domain reference + * intel_aux_display_runtime_put - release an auxiliary power domain reference * @dev_priv: i915 device instance * - * This function drops the auxilliary power domain reference obtained by + * This function drops the auxiliary power domain reference obtained by * intel_aux_display_runtime_get() and might power down the corresponding * hardware block right away if this is the last reference. */ -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] video/fbdev: Always built-in video= cmdline parsing
On Wed, Aug 6, 2014 at 11:43 AM, Daniel Vetter daniel.vet...@ffwll.ch wrote:
In drm/i915 we want to get at the video= cmdline modes even when we
don't have fbdev support enabled, so that users can always override
the kernel's initial mode selection.
But that gives us a direct depency upon the parsing code in the fbdev
subsystem. Since it's so little code just extract these 2 functions
and always build them in.
How much is so little? Think memory-constrained systems.
You can still build it depending on CONFIG_FB or CONFIG_DRM_I915.
diff --git a/drivers/video/fbdev/core/Makefile
b/drivers/video/fbdev/core/Makefile
index fa306538dac2..891c1f890e03 100644
--- a/drivers/video/fbdev/core/Makefile
+++ b/drivers/video/fbdev/core/Makefile
@@ -1,4 +1,4 @@
-obj-y += fb_notify.o
Oh, this is already unconditional. Who are its users?
+obj-y += fb_notify.o fb_cmdline.o
obj-$(CONFIG_FB) += fb.o
fb-y := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
modedb.o fbcvt.o
diff --git a/drivers/video/fbdev/core/fb_cmdline.c
b/drivers/video/fbdev/core/fb_cmdline.c
new file mode 100644
index ..91503a43213e
--- /dev/null
+++ b/drivers/video/fbdev/core/fb_cmdline.c
@@ -0,0 +1,103 @@
+/*
+ * linux/drivers/video/fb_cmdline.c
+ *
+ * Copyright (C) 2014 Intel Corp
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file COPYING in the main directory of this archive
+ * for more details.
+ *
+ * Authors:
+ *Vetter danie.vet...@ffwll.ch
+ */
The above chunk doesn't sound appropriate for extracting existing code...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say programmer or something like that.
-- Linus Torvalds
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Mesa-dev] [PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888
On Sat, Nov 23, 2013 at 2:10 AM, Ville Syrjälä
ville.syrj...@linux.intel.com wrote:
On Fri, Nov 22, 2013 at 03:43:13PM -0800, Keith Packard wrote:
Ville Syrjälä ville.syrj...@linux.intel.com writes:
What is this format anyway? -ENODOCS
Same as MESA_FORMAT_SARGB8 and __DRI_IMAGE_FORMAT_SARGB8 :-)
If its just an srgb version of ARGB, then I wouldn't really want it
in drm_fourcc.h. I expect colorspacy stuff will be handled by various
crtc/plane properties in the kernel so we don't need to encode that
stuff into the fb format.
It's not any different from splitting YUV codes from RGB codes;
Not really. Saying something is YUV (or rather Y'CbCr) doesn't
actually tell you the color space. It just tells you whether the
information is encoded as R+G+B or Y+Cb+Cr. How you convert between
them is another matter. You need to know the gamma, color primaries,
chroma siting for sub-sampled YCbCr formats, etc.
Yep. Fbdev has a separation of type (how pixel values are laid out in memory),
fb_bitfield structs (how tuples are formed into pixels), and visual (how to
interprete the tuples).
The fb_bitfield structs do have RGB-centric names, but you could use them
for e.g. Y, Cb, Cr, and alpha, giving a proper visual. Unfortunately the
YCbCr visuals haven't made it into mainline.
FOURCC unifies all of that in (not so) unique 32-bit IDs.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say programmer or something like that.
-- Linus Torvalds
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] i915: i915_gem: Print a size_t with %z
On Tue, Jun 18, 2013 at 3:26 PM, Fabio Estevam
fabio.este...@freescale.com wrote:
drivers/gpu/drm/i915/i915_gem.c:3129:3: warning: format '%ld' expects
argument of type 'long int', but argument 5 has type 'size_t' [-Wformat]
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3126,7 +3126,7 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object
*obj,
* before evicting everything in a vain attempt to find space.
*/
if (obj-base.size gtt_max) {
- DRM_ERROR(Attempting to bind an object larger than the
aperture: object=%zd %s aperture=%ld\n,
+ DRM_ERROR(Attempting to bind an object larger than the
aperture: object=%zd %s aperture=%zd\n,
size_t is unsigned, so you better change it to %zu.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say programmer or something like that.
-- Linus Torvalds
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
