date:20221108

Hi Maxime,

I ran your v7 patchset on my Pi with Xorg, and the mode switching, as well as
the preferred mode handling, all work really well now!

I just noted that the downstream version of the vc4 driver still has inaccurate
field delays in vc4_crtc.c, which causes vertical lines to appear jagged (like
here: 
https://user-images.githubusercontent.com/4499762/112738569-385c3280-8f64-11eb-83c4-d671537af209.png).
This has been fixed downstream in
https://github.com/raspberrypi/linux/pull/4241/commits/bc093f27bf2613ec93524fdc19e922dd7dd3d800,
but I guess that should be upstreamed separately...?

Anyway, it's unrelated to the changes made in this patchset, so... I'm not sure
if I'm qualified or allowed to do these, but just in case:

Tested-by: Mateusz Kwiatkowski 

(that pretty much applies to parts 19-22 in general, I can respond to those
messages as well if you wish)

Best regards,
Mateusz Kwiatkowski

W dniu 7.11.2022 o 15:16, Maxime Ripard pisze:
> From: Mateusz Kwiatkowski 
>
> Add support for the following composite output modes (all of them are
> somewhat more obscure than the previously defined ones):
>
> - NTSC_443 - NTSC-style signal with the chroma subcarrier shifted to
>   4.43361875 MHz (the PAL subcarrier frequency). Never used for
>   broadcasting, but sometimes used as a hack to play NTSC content in PAL
>   regions (e.g. on VCRs).
> - PAL_N - PAL with alternative chroma subcarrier frequency,
>   3.58205625 MHz. Used as a broadcast standard in Argentina, Paraguay
>   and Uruguay to fit 576i50 with colour in 6 MHz channel raster.
> - PAL60 - 480i60 signal with PAL-style color at normal European PAL
>   frequency. Another non-standard, non-broadcast mode, used in similar
>   contexts as NTSC_443. Some displays support one but not the other.
> - SECAM - French frequency-modulated analog color standard; also have
>   been broadcast in Eastern Europe and various parts of Africa and Asia.
>   Uses the same 576i50 timings as PAL.
>
> Also added some comments explaining color subcarrier frequency
> registers.
>
> Acked-by: Noralf Trønnes 
> Signed-off-by: Mateusz Kwiatkowski 
> Signed-off-by: Maxime Ripard 
>
> ---
> Changes in v6:
> - Support PAL60 again
> ---
>  drivers/gpu/drm/vc4/vc4_vec.c | 111 
> --
>  1 file changed, 107 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c
> index a828fc6fb776..d23dbad3cbf6 100644
> --- a/drivers/gpu/drm/vc4/vc4_vec.c
> +++ b/drivers/gpu/drm/vc4/vc4_vec.c
> @@ -46,6 +46,7 @@
>  #define VEC_CONFIG0_YDEL(x)  ((x) << 26)
>  #define VEC_CONFIG0_CDEL_MASKGENMASK(25, 24)
>  #define VEC_CONFIG0_CDEL(x)  ((x) << 24)
> +#define VEC_CONFIG0_SECAM_STDBIT(21)
>  #define VEC_CONFIG0_PBPR_FIL BIT(18)
>  #define VEC_CONFIG0_CHROMA_GAIN_MASK GENMASK(17, 16)
>  #define VEC_CONFIG0_CHROMA_GAIN_UNITY(0 << 16)
> @@ -76,6 +77,27 @@
>  #define VEC_SOFT_RESET   0x10c
>  #define VEC_CLMP0_START  0x144
>  #define VEC_CLMP0_END0x148
> +
> +/*
> + * These set the color subcarrier frequency
> + * if VEC_CONFIG1_CUSTOM_FREQ is enabled.
> + *
> + * VEC_FREQ1_0 contains the most significant 16-bit half-word,
> + * VEC_FREQ3_2 contains the least significant 16-bit half-word.
> + * 0x8000 seems to be equivalent to the pixel clock
> + * (which itself is the VEC clock divided by 8).
> + *
> + * Reference values (with the default pixel clock of 13.5 MHz):
> + *
> + * NTSC  (3579545.[45] Hz) - 0x21F07C1F
> + * PAL   (4433618.75 Hz)   - 0x2A098ACB
> + * PAL-M (3575611.[888111] Hz) - 0x21E6EFE3
> + * PAL-N (3582056.25 Hz)   - 0x21F69446
> + *
> + * NOTE: For SECAM, it is used as the Dr center frequency,
> + * regardless of whether VEC_CONFIG1_CUSTOM_FREQ is enabled or not;
> + * that is specified as 4406250 Hz, which corresponds to 0x29C71C72.
> + */
>  #define VEC_FREQ3_2  0x180
>  #define VEC_FREQ1_0  0x184
>  
> @@ -118,6 +140,14 @@
>  
>  #define VEC_INTERRUPT_CONTROL0x190
>  #define VEC_INTERRUPT_STATUS 0x194
> +
> +/*
> + * Db center frequency for SECAM; the clock for this is the same as for
> + * VEC_FREQ3_2/VEC_FREQ1_0, which is used for Dr center frequency.
> + *
> + * This is specified as 425 Hz, which corresponds to 0x284BDA13.
> + * That is also the default value, so no need to set it explicitly.
> + */
>  #define VEC_FCW_SECAM_B  0x198
>  #define VEC_SECAM_GAIN_VAL   0x19c
>  
> @@ -197,10 +227,15 @@ enum vc4_vec_tv_mode_id {
>   VC4_VEC_TV_MODE_NTSC_J,
>   VC4_VEC_TV_MODE_PAL,
>   VC4_VEC_TV_MODE_PAL_M,
> + VC4_VEC_TV_MODE_NTSC_443,
> + VC4_VEC_TV_MODE_PAL_60,
> + VC4_VEC_TV_MODE_PAL_N,
> + VC4_VEC_TV_MODE_SECAM,
>  };
>  
>  struct vc4_vec_tv_mode {
>   unsigned int mode;
> + u16 expected_htotal;
>   u32 config0;
>   u32

Re: [PATCH 04/10] vfio: Move storage of allow_unsafe_interrupts to vfio_main.c

On Tue, Nov 08, 2022 at 03:55:20PM -0700, Alex Williamson wrote:

> > > So why exactly isn't this an issue for VDPA?  Are we just burying our
> > > head in the sand that such platforms exists and can still be useful
> > > given the appropriate risk vs reward trade-off?  
> > 
> > Simply that nobody has asked for it, and might never ask for it. This
> > is all support for old platforms, and there just doesn't seem to be a
> > "real" use case for very new (and actually rare) NIC hardware stuck
> > into ancient platforms with this security problem.
> 
> vIOMMU support for interrupt remapping is relatively new, the nesting
> case is important as well.

This is where we got hit. In the end we fixed the qemu..

> > I'd be much more comfortable with this as a system wide iommufd flag
> > if we also tied it to do some demonstration of privilege - eg a
> > requirement to open iommufd with CAP_SYS_RAWIO for instance.
> 
> Which is not compatible to existing use cases, which is also why we
> can't invent some way to allow some applications to run without CPU
> mitigations, while requiring it for others as a baseline.

Isn't it? Didn't we learn that libvirt runs as root and will open and
pass the iommufd as root?

> > That is the usual protocol for these kinds of insecurities..
> 
> Hmm, is it?

I think so. At least you should have something to shut down an
insecure feature in kernel lockdown modes. CAP_SYS_RAWIO is a simple
way to do it.

> > I think right now we can leave this as-is and we can wait for some
> > more information to decide how best to proceed.
> 
> It's certainly not acceptable in the latest proposal, iommufd consumes
> an option set by another module and when that module goes away, so does
> any claim of compatibility.  The code becomes dead and the feature not
> present.  The option doesn't belong on the vfio module.  Do we need a
> vfio-iommufd module to host it?  Thanks,

I don't know, as I said in the other email, these little things need
work and discussion to resolve. We need to recheck the security stuff
against the 2022 kernel where things have changed. We don't need to do
it all right now.

People who want allow_unsafe_interrupts to work will simply not set
VFIO_CONTAINER=n at this time. Same with P2P, vfio-no-iommu and any
other gaps we haven't discovered.

vfio-iommufd seems like overkill, I think your first suggestion to put
in vfio.ko was more practical.

My only doubt is if we should make it system wide for everything - and
I'm just a bit uncomfortable with that from a security POV. But maybe
I don't quite know exactly what the risks are.

Jason

Re: [PATCH v2 10/11] vfio: Make vfio_container optionally compiled

On Tue, Nov 08, 2022 at 03:28:31PM -0700, Alex Williamson wrote:

> Perhaps this should have been obvious, but I'm realizing that
> vfio-noiommu mode is completely missing without VFIO_CONTAINER, which
> seems a barrier to deprecating VFIO_CONTAINER and perhaps makes it a

Yes, it is the same as the allow_unsafe_interrupts - it is something
that currently goes missing if you turn off VFIO_CONTAINER.

This seems straightforward enough to resolve in a followup, we mostly
just need someone with an existing no-iommu application to test
compatability against. Keeping it working with the device cdev will
also be a bit interesting. If you have or know about some application
I can try to make a patch.

> question whether IOMMUFD should really be taking over /dev/vfio/vfio.
> No-iommu mode has users.  

I view VFIO_CONTAINER=n as a process. An aspiration we can work
toward.

At this point there are few places that might want to use it. Android
perhaps, for example. It is also useful for testing. One of the main
values is you can switch the options and feed the kernel into an
existing test environment and see what happens. This is how we are
able to quickly get s390 mdev testing, for instance.

We are not going to get to a widely useful VFIO_CONTAINER=n if we
don't have a target that people can test against and evaluate what
compatability gaps may exist.

So, everytime we find something like this - let's think about how can
we make iommufd compatibility handle it and not jump straight to
giving up :)

I'm kind of thinking v6.4 might be a reasonable kernel target when we
might have closed off enough things.

Jason

[PATCH v2] drm: xlnx: Fix return type of zynqmp_dp_bridge_mode_valid

2022-11-08 Thread Nathan Chancellor

From: Nathan Huckleberry 

The mode_valid field in drm_bridge_helper_funcs is expected to be of
type
enum drm_mode_status (* mode_valid) (struct drm_bridge *bridge,
 struct drm_display_mode *mode);

The mismatched return type breaks forward edge kCFI since the underlying
function definition does not match the function hook definition. A new
warning in clang will catch this at compile time:

  drivers/gpu/drm/xlnx/zynqmp_dp.c:1573:16: error: incompatible function 
pointer types initializing 'enum drm_mode_status (*)(struct drm_bridge *, const 
struct drm_display_info *, const struct drm_display_mode *)' with an expression 
of type 'int (struct drm_bridge *, const struct drm_display_info *, const 
struct drm_display_mode *)' 
[-Werror,-Wincompatible-function-pointer-types-strict]
  .mode_valid = zynqmp_dp_bridge_mode_valid,
^~~
  1 error generated.

The return type of zynqmp_dp_bridge_mode_valid should be changed from
int to enum drm_mode_status.

Reported-by: Dan Carpenter 
Link: https://github.com/ClangBuiltLinux/linux/issues/1703
Link: https://github.com/ClangBuiltLinux/linux/issues/1750
Signed-off-by: Nathan Huckleberry 
Reviewed-by: Laurent Pinchart 
[nathan: Rebase on drm-misc-next and fix conflicts
 Add note about new clang warning]
Signed-off-by: Nathan Chancellor 
---

Please consider picking this up so that it makes 6.2.

v2:
  - Take over for Nathan, as he is busy with other matters.
  - Rebase on drm-misc-next and resolve conflicts.
  - Add a note about new clang warning that will catch this issue at
compile time.

v1: https://lore.kernel.org/20220913205600.155172-1-nh...@google.com/

 drivers/gpu/drm/xlnx/zynqmp_dp.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
index 7c9ae167eac7..0a7b466446fb 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
@@ -1362,9 +1362,10 @@ static void zynqmp_dp_bridge_detach(struct drm_bridge 
*bridge)
zynqmp_dp_aux_cleanup(dp);
 }
 
-static int zynqmp_dp_bridge_mode_valid(struct drm_bridge *bridge,
-  const struct drm_display_info *info,
-  const struct drm_display_mode *mode)
+static enum drm_mode_status
+zynqmp_dp_bridge_mode_valid(struct drm_bridge *bridge,
+   const struct drm_display_info *info,
+   const struct drm_display_mode *mode)
 {
struct zynqmp_dp *dp = bridge_to_dp(bridge);
int rate;

base-commit: 1a0257c352638916fdaffaac2ddedb8e049312f3
-- 
2.38.1

[PATCH] drm/i915/dg2: Drop force_probe requirement

2022-11-08 Thread Matt Roper

DG2 has been very usable for a while now, and all of the uapi changes
related to fundamental platform usage have been finalized.  Recent CI
results have also been healthy, so we're ready to drop the force_probe
requirement and enable the platform by default.

Cc: Rodrigo Vivi 
Cc: Tvrtko Ursulin 
Cc: Joonas Lahtinen 
Cc: Jani Nikula 
Signed-off-by: Matt Roper 
---

There was some recent offline discussion questioning whether we'd fully
identified the root cause of some historic CI failures, or whether it
was possible we might still have a bug lurking somewhere causing
sporadic failures.  Let's use this patch to centralize discussion about
any remaining concerns and make sure they're addressed before we apply
this.

 drivers/gpu/drm/i915/i915_pci.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 211913be40ce..0866300243aa 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -1078,7 +1078,6 @@ static const struct intel_device_info dg2_info = {
XE_LPD_FEATURES,
.__runtime.cpu_transcoder_mask = BIT(TRANSCODER_A) | BIT(TRANSCODER_B) |
   BIT(TRANSCODER_C) | BIT(TRANSCODER_D),
-   .require_force_probe = 1,
 };

 static const struct intel_device_info ats_m_info = {
-- 
2.38.1

Re: [PATCH 04/10] vfio: Move storage of allow_unsafe_interrupts to vfio_main.c

2022-11-08 Thread Alex Williamson

On Mon, 7 Nov 2022 14:45:59 -0400
Jason Gunthorpe  wrote:

> On Mon, Nov 07, 2022 at 11:05:08AM -0700, Alex Williamson wrote:
> 
> > After further consideration... I don't think the option on vfio-main
> > makes sense, basically for the same reason that the original option
> > existed on the IOMMU backend rather than vfio-core.  The option
> > describes a means to relax a specific aspect of IOMMU isolation, which
> > makes more sense to expose via the IOMMU provider, imo.  For example,
> > vfio-main cannot generate an equivalent error message as provided in
> > type1 today, it's too far removed from the IOMMU feature support.  
> 
> vfio-main can do it, we just have to be strict that the EPERM code is
> always going to be this case.

This doesn't seem very practical.

> > > > If vdpa doesn't allow full device access such that it can guarantee
> > > > that a device cannot generate a DMA that can spoof MSI, then it
> > > > sounds like the flag we pass when attaching a device to iommfd
> > > > should to reflect this difference in usage.
> > > 
> > > VDPA allows arbitary DMA just like VFIO. At most VDPA limits the MMIO
> > > touches.  
> >
> > So why exactly isn't this an issue for VDPA?  Are we just burying our
> > head in the sand that such platforms exists and can still be useful
> > given the appropriate risk vs reward trade-off?  
> 
> Simply that nobody has asked for it, and might never ask for it. This
> is all support for old platforms, and there just doesn't seem to be a
> "real" use case for very new (and actually rare) NIC hardware stuck
> into ancient platforms with this security problem.

vIOMMU support for interrupt remapping is relatively new, the nesting
case is important as well.

> So I'd rather leave this in the past than carry forward a security
> exception as some ongoing 1st class thing.
> 
> > > and IMHO we don't actually want to enable this more
> > > widely. So I don't want to see a global kernel wide flag at this point
> > > until we get reason to make more than just VFIO insecure.  
> > 
> > But this brings into question the entire existence of the opt-in.  Do
> > we agree that there are valid use cases for such an option?  
> 
> I think it is something VFIO has historically allowed and I think we
> can continue to allow it, but I don't think we should encourage its
> use or encourage it to propogate to wider areas given that the
> legitimate use cases are focused on fairly old hardware at this point.
> 
> So, I'd rather wait for someone to ask for it, and explain why they
> need to use a combination of stuff where we need to have a true global
> option.
> 
> > Unlike things like ACS overrides, lack of interrupt isolation really
> > requires a malicious actor.  We're not going to inadvertently overlap
> > DMA to interrupt addresses like we might to a non-isolated MMIO ranges.
> > Therefore an admin can make a reasonable determination relative to the
> > extent to which the userspace is trusted.  This is not unlike opt-outs
> > to CPU vulnerability mitigation imo, there are use cases where the
> > performance or functionality is more important than the isolation.
> > Hand waving this away as a vfio-unique insecurity is a bad precedent
> > for iommufd.  
> 
> I agree with this, which is why I think it should come from the actual
> user facing subsystem not be a system wide flag. The "is userspace
> trusted" for VFIO may be quite different than from VDPA or whatever
> else comes next.
> 
> I'd be much more comfortable with this as a system wide iommufd flag
> if we also tied it to do some demonstration of privilege - eg a
> requirement to open iommufd with CAP_SYS_RAWIO for instance.

Which is not compatible to existing use cases, which is also why we
can't invent some way to allow some applications to run without CPU
mitigations, while requiring it for others as a baseline.

> That is the usual protocol for these kinds of insecurities..

Hmm, is it?

> I think right now we can leave this as-is and we can wait for some
> more information to decide how best to proceed.

It's certainly not acceptable in the latest proposal, iommufd consumes
an option set by another module and when that module goes away, so does
any claim of compatibility.  The code becomes dead and the feature not
present.  The option doesn't belong on the vfio module.  Do we need a
vfio-iommufd module to host it?  Thanks,

Alex

Re: [PATCH v2 10/11] vfio: Make vfio_container optionally compiled

2022-11-08 Thread Alex Williamson

On Mon,  7 Nov 2022 20:52:54 -0400
Jason Gunthorpe  wrote:

> Add a kconfig CONFIG_VFIO_CONTAINER that controls compiling the container
> code. If 'n' then only iommufd will provide the container service. All the
> support for vfio iommu drivers, including type1, will not be built.
> 
> This allows a compilation check that no inappropriate dependencies between
> the device/group and container have been created.
> 
> Tested-by: Nicolin Chen 
> Signed-off-by: Jason Gunthorpe 
> ---
>  drivers/vfio/Kconfig  | 35 +++
>  drivers/vfio/Makefile |  4 +--
>  drivers/vfio/vfio.h   | 65 +++
>  3 files changed, 91 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
> index 1118d322eec97d..286c1663bd7564 100644
> --- a/drivers/vfio/Kconfig
> +++ b/drivers/vfio/Kconfig
> @@ -3,8 +3,8 @@ menuconfig VFIO
>   tristate "VFIO Non-Privileged userspace driver framework"
>   select IOMMU_API
>   depends on IOMMUFD || !IOMMUFD
> - select VFIO_IOMMU_TYPE1 if MMU && (X86 || S390 || ARM || ARM64)
>   select INTERVAL_TREE
> + select VFIO_CONTAINER if IOMMUFD=n
>   help
> VFIO provides a framework for secure userspace device drivers.
> See Documentation/driver-api/vfio.rst for more details.
> @@ -12,6 +12,18 @@ menuconfig VFIO
> If you don't know what to do here, say N.
>  
>  if VFIO
> +config VFIO_CONTAINER
> + bool "Support for the VFIO container /dev/vfio/vfio"
> + select VFIO_IOMMU_TYPE1 if MMU && (X86 || S390 || ARM || ARM64)
> + default y
> + help
> +   The VFIO container is the classic interface to VFIO for establishing
> +   IOMMU mappings. If N is selected here then IOMMUFD must be used to
> +   manage the mappings.
> +
> +   Unless testing IOMMUFD say Y here.
> +
> +if VFIO_CONTAINER
>  config VFIO_IOMMU_TYPE1
>   tristate
>   default n
> @@ -21,16 +33,6 @@ config VFIO_IOMMU_SPAPR_TCE
>   depends on SPAPR_TCE_IOMMU
>   default VFIO
>  
> -config VFIO_SPAPR_EEH
> - tristate
> - depends on EEH && VFIO_IOMMU_SPAPR_TCE
> - default VFIO
> -
> -config VFIO_VIRQFD
> - tristate
> - select EVENTFD
> - default n
> -
>  config VFIO_NOIOMMU
>   bool "VFIO No-IOMMU support"
>   help


Perhaps this should have been obvious, but I'm realizing that
vfio-noiommu mode is completely missing without VFIO_CONTAINER, which
seems a barrier to deprecating VFIO_CONTAINER and perhaps makes it a
question whether IOMMUFD should really be taking over /dev/vfio/vfio.
No-iommu mode has users.  Thanks,

Alex

Re: [Nouveau] [PATCH v7 22/23] drm/vc4: vec: Add support for more analog TV standards

Hi Lukas, Maxime and everyone,

W dniu 8.11.2022 o 14:17, Lukas Satin pisze:
> They are important for retrogaming and connecting TV out to CRT TV or using
> emulator.
>
> I have PS1 that is using PAL-60 for example.
>
> Can you add 240p and 288p non-interlaced modes for NTSC and PAL, please?

To add progressive mode support, at least for the VC4/VEC device that's used
on the Raspberry Pi, all that's necessary is a patch like:

--- a/drivers/gpu/drm/vc4/vc4_vec.c
+++ b/drivers/gpu/drm/vc4/vc4_vec.c
@@ -623,7 +623,9 @@ static void vc4_vec_encoder_enable(struct drm_encoder 
*encoder,
VEC_WRITE(VEC_CLMP0_START, 0xac);
VEC_WRITE(VEC_CLMP0_END, 0xec);
VEC_WRITE(VEC_CONFIG2,
- VEC_CONFIG2_UV_DIG_DIS | VEC_CONFIG2_RGB_DIG_DIS);
+ VEC_CONFIG2_UV_DIG_DIS |
+ VEC_CONFIG2_RGB_DIG_DIS |
+ ((adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) ? 0 : 
VEC_CONFIG2_PROG_SCAN));
VEC_WRITE(VEC_CONFIG3, VEC_CONFIG3_HORIZ_LEN_STD);
VEC_WRITE(VEC_DAC_CONFIG, vec->variant->dac_config);
 

and then you can just add custom modes, for example within Xorg:

xrandr --newmode 720x240 13.5 720 736 800 858 240 243 246 262
xrandr --newmode 720x288 13.5 720 740 804 864 288 290 293 312

Note that the pixel aspect ratio will be all over the place - unfortunately this
is necessary at the driver level, because VC4's VEC does not support pixel
clocks other than 13.5 MHz. However, you can fix it by running something like
"xrandr --scale-from 320x240" or "xrandr --scale-from 384x288". Other (non-X)
applications would need to be adapted to similarly configure DRM scaling.

I'm not sure if Maxime wants to introduce any more code like the patch above to
facilitate progressive scan support, though (@Maxime: feel free to grab the code
from above or anything else from https://github.com/raspberrypi/linux/pull/4406
if you do, however!). We talked recently that the priority is to finally merge
existing functionality first, see this message:
https://lore.kernel.org/dri-devel/20221027115822.5vd3fqlcpy4gfq5v@houat/

I'm willing to post a couple of follow-up patches to improve things like
support for progressive modes or exotic TV norms (such as PAL-M-50 or PAL-N-60)
within the VC4 driver once this patchset lands - but I agree with Maxime's point
to focus on merging existing functionality first.

> Lukas

Best regards,
Mateusz Kwiatkowski

> On Mon, Nov 7, 2022 at 3:19 PM Maxime Ripard  wrote:
>
> From: Mateusz Kwiatkowski 
>
> Add support for the following composite output modes (all of them are
> somewhat more obscure than the previously defined ones):
>
> - NTSC_443 - NTSC-style signal with the chroma subcarrier shifted to
>   4.43361875 MHz (the PAL subcarrier frequency). Never used for
>   broadcasting, but sometimes used as a hack to play NTSC content in PAL
>   regions (e.g. on VCRs).
> - PAL_N - PAL with alternative chroma subcarrier frequency,
>   3.58205625 MHz. Used as a broadcast standard in Argentina, Paraguay
>   and Uruguay to fit 576i50 with colour in 6 MHz channel raster.
> - PAL60 - 480i60 signal with PAL-style color at normal European PAL
>   frequency. Another non-standard, non-broadcast mode, used in similar
>   contexts as NTSC_443. Some displays support one but not the other.
> - SECAM - French frequency-modulated analog color standard; also have
>   been broadcast in Eastern Europe and various parts of Africa and Asia.
>   Uses the same 576i50 timings as PAL.
>
> Also added some comments explaining color subcarrier frequency
> registers.
>
> Acked-by: Noralf Trønnes 
> Signed-off-by: Mateusz Kwiatkowski 
> Signed-off-by: Maxime Ripard 
>
> ---
> Changes in v6:
> - Support PAL60 again
> ---
>  drivers/gpu/drm/vc4/vc4_vec.c | 111 
> --
>  1 file changed, 107 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c
> index a828fc6fb776..d23dbad3cbf6 100644
> --- a/drivers/gpu/drm/vc4/vc4_vec.c
> +++ b/drivers/gpu/drm/vc4/vc4_vec.c
> @@ -46,6 +46,7 @@
>  #define VEC_CONFIG0_YDEL(x)((x) << 26)
>  #define VEC_CONFIG0_CDEL_MASK  GENMASK(25, 24)
>  #define VEC_CONFIG0_CDEL(x)((x) << 24)
> +#define VEC_CONFIG0_SECAM_STD  BIT(21)
>  #define VEC_CONFIG0_PBPR_FIL   BIT(18)
>  #define VEC_CONFIG0_CHROMA_GAIN_MASK   GENMASK(17, 16)
>  #define VEC_CONFIG0_CHROMA_GAIN_UNITY  (0 << 16)
> @@ -76,6 +77,27 @@
>  #define VEC_SOFT_RESET 0x10c
>  #define VEC_CLMP0_START0x144
>  #define VEC_CLMP0_END  0x148
> +
> +/*
> + * These set the color subcarrier frequency
> + * if VEC_CONFIG1_CUSTOM_FREQ is enabled.
> + *
> + * VEC_FREQ1_0 contains the most significant 16-bit half-wor

Re: [PATCH v2 0/7] drm: Review of mode copies

On Tue, Nov 8, 2022 at 10:54 AM Daniel Vetter  wrote:
>
> On Mon, Nov 07, 2022 at 09:25:38PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä 
> >
> > Repost of the stragglers from
> > https://patchwork.freedesktop.org/series/100393/
> >
> > Note that I didn't rerun the cocci stuff, nor have I had
> > time to come up with something to inluce the cocci scripts
> > in the tree. So it's possible this is missing some new
> > things that have snuck in the meantime.
> >
> > Ville Syrjälä (7):
> >   drm/amdgpu: Use drm_mode_init() for on-stack modes
> >   drm/hisilicon: Use drm_mode_init() for on-stack modes
> >   drm/msm: Use drm_mode_init() for on-stack modes
> >   drm/msm: Use drm_mode_copy()
> >   drm/mtk: Use drm_mode_init() for on-stack modes
> >   drm/rockchip: Use drm_mode_copy()
> >   drm/sti: Use drm_mode_copy()
>
> On the series:
>
> Reviewed-by: Daniel Vetter 
>
> Imo ping Alex/Rob and stuff everything left into drm-misc-next.

Acked-by: Alex Deucher 
Feel free to take this series through whatever tree makes sense.  If
you want me to pick up the amdgpu change directly, I can do that too.

Alex

> -Daniel
>
> >
> >  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c| 3 ++-
> >  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c | 2 +-
> >  drivers/gpu/drm/mediatek/mtk_hdmi.c  | 2 +-
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 7 +--
> >  drivers/gpu/drm/msm/dp/dp_display.c  | 2 +-
> >  drivers/gpu/drm/rockchip/cdn-dp-core.c   | 2 +-
> >  drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +-
> >  drivers/gpu/drm/rockchip/rk3066_hdmi.c   | 2 +-
> >  drivers/gpu/drm/sti/sti_dvo.c| 2 +-
> >  drivers/gpu/drm/sti/sti_hda.c| 2 +-
> >  drivers/gpu/drm/sti/sti_hdmi.c   | 2 +-
> >  11 files changed, 16 insertions(+), 12 deletions(-)
> >
> > --
> > 2.37.4
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

Re: [RFC PATCH 1/3] drm: Introduce color fill properties for drm plane

2022-11-08 Thread Sebastian Wick

On Tue, Nov 8, 2022 at 7:51 PM Simon Ser  wrote:
>
> cc'ing Pekka and wayland-devel for userspace devs feedback on the new uAPI.
>
> On Saturday, October 29th, 2022 at 14:08, Dmitry Baryshkov 
>  wrote:
>
> > On 29/10/2022 01:59, Jessica Zhang wrote:
> > > Add support for COLOR_FILL and COLOR_FILL_FORMAT properties for
> > > drm_plane. In addition, add support for setting and getting the values
> > > of these properties.
> > >
> > > COLOR_FILL represents the color fill of a plane while COLOR_FILL_FORMAT
> > > represents the format of the color fill. Userspace can set enable solid
> > > fill on a plane by assigning COLOR_FILL to a uint64_t value, assigning
> > > the COLOR_FILL_FORMAT property to a uint32_t value, and setting the
> > > framebuffer to NULL.
> > >
> > > Signed-off-by: Jessica Zhang 
> >
> > Planes report supported formats using the drm_mode_getplane(). You'd
> > also need to tell userspace, which formats are supported for color fill.
> > I don't think one supports e.g. YV12.
> >
> > A bit of generic comment for the discussion (this is an RFC anyway).
> > Using color_fill/color_fill_format properties sounds simple, but this
> > might be not generic enough. Limiting color_fill to 32 bits would
> > prevent anybody from using floating point formats (e.g.
> > DRM_FORMAT_XRGB16161616F, 64-bit value). Yes, this can be solved with
> > e.g. using 64-bit for the color_fill value, but then this doesn't sound
> > extensible too much.
> >
> > So, a question for other hardware maintainers. Do we have hardware that
> > supports such 'color filled' planes? Do we want to support format
> > modifiers for filling color/data? Because what I have in mind is closer
> > to the blob structure, which can then be used for filling the plane:
> >
> > struct color_fill_blob {
> >  u32 pixel_format;
> >  u64 modifiers4];
> >  u32 pixel_data_size; // fixme: is this necessary?
> >  u8 pixel_data[];
> > };
> >
> > And then... This sounds a lot like a custom framebuffer.
> >
> > So, maybe what should we do instead is to add new DRM_MODE_FB_COLOR_FILL
> > flag to the framebuffers, which would e.g. mean that the FB gets stamped
> > all over the plane. This would also save us from changing if (!fb)
> > checks all over the drm core.
> >
> > Another approach might be using a format modifier instead of the FB flag.
> >
> > What do you think?
>
> First off, we only need to represent the value of a single pixel here. So I'm
> not quite following why we need format modifiers. Format modifiers describe 
> how
> pixels are laid out in memory. Since there's a single pixel described, this
> is non-sensical to me, the format modifier is always LINEAR.
>
> Then, I can understand why putting the pixel_format in there is tempting to
> guarantee future extensibility, but it also adds complexity. For instance, how
> does user-space figure out which formats can be used for COLOR_FILL? Can
> user-space use any format supported by the plane? What does it mean for
> multi-planar formats? Do we really want the kernel to have conversion logic 
> for
> all existing formats? Do we need to also add a new read-only blob prop to
> indicate supported COLOR_FILL formats?
>
> We've recently-ish standardized a new Wayland protocol [1] which has the same
> purpose as this new kernel uAPI. The conclusion there was that using 32-bit
> values for each channel (R, G, B, A) would be enough for almost all use-cases.
> The driver can convert these high-precision values to what the hardware 
> expects.
> The only concern was about sending values outside of the [0.0, 1.0] range,
> which may have HDR use-cases.
>
> So, there are multiple ways to go about this. I can think of:
>
> - Put "RGBA32" in the name of the prop, and if we ever need a different
>   color format, pick a different name.
> - Define a struct with an enum of possible fill kinds:
>   #define FILL_COLOR_RGBA32 1
>   #define FILL_COLOR_F32 2
>   struct color_fill_blob { u32 kind; u8 data[]; };
> - Define a struct with a version and RGBA values:
>   struct color_fill_blob { u32 version; u32 rgba[4]; };
>   If we need to add more formats later, or new metadata:
>   struct color_fill_blob2 { u32 version; /* new fields */ };
>   where version must be set to 2.
> - Define a struct with a "pixel_format" prop, but force user-space to use a
>   fixed format for now. Later, if we need another format, add a new prop to
>   advertise supported formats.
> - More complicated solutions, e.g. advertise the list of supported formats 
> from
>   the start.
>
> [1]: 
> https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/104
>

Agreeing with most of what you said here. However, what's the idea
behind a format anyway? The 4 values provided here are fed directly
into the color pipeline which seems to define the color channels it's
working on as RGBA (or doesn't define anything at all). The only
reason I can think of is that hardware might support only ingesting
values either in a format wi

Re: [Nouveau] [PATCH v7 06/23] drm/modes: Add a function to generate analog display modes

Hi Lukas,

W dniu 8.11.2022 o 14:28, Lukas Satin pisze:
> Hi, your statement:
>
> "However, analog display usually have fairly loose timings requirements,
> the only discrete parameters being the total number of lines and pixel
> clock frequency."
>
> Please do not make it as a rule. You said yourself: "usually". Arcade CRT
> have more loose timings, but professional broadcast TV's such as Sony PVM,
> Sony BVM, JVC. These cost tens of thousand dollars back in the day. Now they
> are affordable for gamers. I just solved issue in Retroarch, CRT Switchres
> library here: https://github.com/antonioginer/switchres/issues/96

I think I'm partially to blame for this wording.
See this message and the surrounding thread:
https://lore.kernel.org/dri-devel/6d1dfaad-7310-a596-34dd-4a6d9aa95...@gmail.com/

A lot of composite video equipment routinely violated the reference spec.
For example, CGA and Apple II output singal that had 15699.76 Hz horizontal
sync and 59.92 Hz vertical sync instead of the regular 15734.26 Hz / 59.94 Hz.
Values for Famicom/NES are 15745.98 Hz / 60.10 Hz (and the last line is slightly
shorter than all other ones at that). And I'm pretty sure these will display
just fine on a PVM.

Of course you can't just output 14 kHz / 70 Hz and expect it to work, there are
constraints of display capabilities. But the point of that discussion, which
culminated in the wording you see in the code, was that it does not need to be
precise down to every clock cycle as you would normally expect in the digital
world.

> This model is quite common among retrogamers and on Reddit.
>
> Some developers do not test it properly.
>
> This model requires exact number of lines.
>
> For Switchres we came up with these ranges:
> crt_range0 15625-15750, 49.50-65.00, 2.000, 4.700, 8.000, 0.064, 
> 0.192, 1.024, 1, 1, 192, 288, 0, 0
> crt_range1 15625.00-15625.00, 50.00-50.00, 1.500, 4.700, 5.800, 
> 0.064, 0.160, 1.056, 1, 1, 0, 0, 448, 576
> crt_range2 15734.26-15734.26, 59.94-59.94, 1.500, 4.700, 4.700, 
> 0.191, 0.191, 0.953, 1, 1, 0, 0, 448, 480
> crt_range0 is default, more loose definition for MAME emulators. crt_range1 
> is PAL and crt_range2 is NTSC.
>
> Yes, this model does support both NTSC and PAL.
>
> Does your driver or library support that?
>
> For example old driver in Windows 7 with NVIDIA 2007 driver on Geforce 7600
> can support both NTSC and PAL and these are being switched automatically by
> the resolution you choose. So in desktop properties, you change to 640x480
> and it will switch TV chipset to NTSC 480i. Then you change to 720x576 and it
> will switch TV chipset to PAL 576i.
>
> It would be preferred if advanced users could set up these numbers from a
> commandline during a runtime, so it would depend on the app being used.

As far as I understand the patch, this is exactly how it works right now. The
function you're commenting on is only used for generating the "default" modes.

> Lukas

Best regards,
Mateusz Kwiatkowski

> On Mon, Nov 7, 2022 at 3:17 PM Maxime Ripard  wrote:
>
> Multiple drivers (meson, vc4, sun4i) define analog TV 525-lines and
> 625-lines modes in their drivers.
>
> Since those modes are fairly standard, and that we'll need to use them
> in more places in the future, it makes sense to move their definition
> into the core framework.
>
> However, analog display usually have fairly loose timings requirements,
> the only discrete parameters being the total number of lines and pixel
> clock frequency. Thus, we created a function that will create a display
> mode from the standard, the pixel frequency and the active area.
>
> Signed-off-by: Maxime Ripard 
>
> ---
> Changes in v6:
> - Fix typo
>
> Changes in v4:
> - Reworded the line length check comment
> - Switch to HZ_PER_KHZ in tests
> - Use previous timing to fill our mode
> - Move the number of lines check earlier
> ---
>  drivers/gpu/drm/drm_modes.c| 474 
> +
>  drivers/gpu/drm/tests/Makefile |   1 +
>  drivers/gpu/drm/tests/drm_modes_test.c | 144 ++
>  include/drm/drm_modes.h|  17 ++
>  4 files changed, 636 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 5d4ac79381c4..71c050c3ee6b 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -116,6 +116,480 @@ void drm_mode_probed_add(struct drm_connector 
> *connector,
>  }
>  EXPORT_SYMBOL(drm_mode_probed_add);
>
> +enum drm_mode_analog {
> +   DRM_MODE_ANALOG_NTSC, /* 525 lines, 60Hz */
> +   DRM_MODE_ANALOG_PAL, /* 625 lines, 50Hz */
> +};
> +
> +/*
> + * The timings come from:
> + * - 
> https://web.archive.org/web/20220406232708/http://www.kolumbus.fi/pami1/video/pal_ntsc.html
> + * - 
> https://web.archive.org/web/20220406124914/http://mar

Re: [PATCH v7 15/23] drm/modes: Introduce more named modes

Hi Noralf,

W dniu 8.11.2022 o 10:38, Noralf Trønnes pisze:
>
> Den 07.11.2022 19.03, skrev Noralf Trønnes:
>>
>> Den 07.11.2022 15.16, skrev Maxime Ripard:
>>> Now that we can easily extend the named modes list, let's add a few more
>>> analog TV modes that were used in the wild, and some unit tests to make
>>> sure it works as intended.
>>>
>>> Signed-off-by: Maxime Ripard 
>>>
>>> ---
>>> Changes in v6:
>>> - Renamed the tests to follow DRM test naming convention
>>>
>>> Changes in v5:
>>> - Switched to KUNIT_ASSERT_NOT_NULL
>>> ---
>>>  drivers/gpu/drm/drm_modes.c |  2 +
>>>  drivers/gpu/drm/tests/drm_client_modeset_test.c | 54 
>>> +
>>>  2 files changed, 56 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
>>> index 49441cabdd9d..17c5b6108103 100644
>>> --- a/drivers/gpu/drm/drm_modes.c
>>> +++ b/drivers/gpu/drm/drm_modes.c
>>> @@ -2272,7 +2272,9 @@ struct drm_named_mode {
>>>  
>>>  static const struct drm_named_mode drm_named_modes[] = {
>>> NAMED_MODE("NTSC", 13500, 720, 480, DRM_MODE_FLAG_INTERLACE, 
>>> DRM_MODE_TV_MODE_NTSC),
>>> +   NAMED_MODE("NTSC-J", 13500, 720, 480, DRM_MODE_FLAG_INTERLACE, 
>>> DRM_MODE_TV_MODE_NTSC_J),
>>> NAMED_MODE("PAL", 13500, 720, 576, DRM_MODE_FLAG_INTERLACE, 
>>> DRM_MODE_TV_MODE_PAL),
>>> +   NAMED_MODE("PAL-M", 13500, 720, 480, DRM_MODE_FLAG_INTERLACE, 
>>> DRM_MODE_TV_MODE_PAL_M),
>>>  };
>> I'm now having second thoughts about the tv_mode commandline option. Can
>> we just add all the variants to this table and drop the tv_mode option?
>> IMO this will be more user friendly and less confusing.
>>
> One downside of this is that it's not possible to force connector status
> when using named modes, but I think it would be better to have a force
> option than a tv_mode option. A lot of userspace treats unknown status
> as disconnected.
>
> Anyone know if it's possible to set the connector status sysfs file
> using a udev rule?
>
> Noralf.

I think that leaving named modes only would be a bit limiting. There are use
cases for custom modes, e.g. we might want progressive 240p "NTSC" (like 80s/90s
home computers and video game consoles) or the modes with non-13.5MHz pixel
clock that Geert requested with Amiga in mind.

I'm not sure if the current cmdline-to-drm_mode conversion is flexible enough
to meaningfully facilitate those, but we're at least getting the syntax down.

Best regards,
Mateusz Kwiatkowski

Re: [PATCH v2 08/18] dt-bindings: msm: dsi-controller-main: Document clocks on a per compatible basis

On 08/11/2022 00:56, Bryan O'Donoghue wrote:
> Each compatible has a different set of clocks which are associated with it.
> Add in the list of clocks for each compatible.
> 
> Cc: Rob Clark 
> Cc: Abhinav Kumar 
> Cc: Dmitry Baryshkov 
> Cc: Sean Paul 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: Rob Herring 
> Cc: Krzysztof Kozlowski 
> Cc: linux-arm-...@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: freedr...@lists.freedesktop.org
> Cc: devicet...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Signed-off-by: Bryan O'Donoghue 
> ---
>  .../display/msm/dsi-controller-main.yaml  | 177 +++---
>  1 file changed, 150 insertions(+), 27 deletions(-)
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml 
> b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> index 9db3e63acda3d..c975df0ca22fc 100644
> --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> @@ -9,24 +9,22 @@ title: Qualcomm Display DSI controller
>  maintainers:
>- Krishna Manikandan 
>  
> -allOf:
> -  - $ref: "../dsi-controller.yaml#"
> -
>  properties:
>compatible:
> -enum:
> -  - qcom,dsi-ctrl-6g-qcm2290
> -  - qcom,mdss-dsi-ctrl
> -  - qcom,mdss-dsi-ctrl-apq8064
> -  - qcom,mdss-dsi-ctrl-msm8916
> -  - qcom,mdss-dsi-ctrl-msm8974
> -  - qcom,mdss-dsi-ctrl-msm8996
> -  - qcom,mdss-dsi-ctrl-sc7180
> -  - qcom,mdss-dsi-ctrl-sc7280
> -  - qcom,mdss-dsi-ctrl-sdm630
> -  - qcom,mdss-dsi-ctrl-sdm660
> -  - qcom,mdss-dsi-ctrl-sdm845
> -  - qcom,mdss-dsi-ctrl-sm8250
> +items:
> +  - enum:
> +  - qcom,dsi-ctrl-6g-qcm2290
> +  - qcom,mdss-dsi-ctrl-apq8064
> +  - qcom,mdss-dsi-ctrl-msm8916
> +  - qcom,mdss-dsi-ctrl-msm8974
> +  - qcom,mdss-dsi-ctrl-msm8996
> +  - qcom,mdss-dsi-ctrl-sc7180
> +  - qcom,mdss-dsi-ctrl-sc7280
> +  - qcom,mdss-dsi-ctrl-sdm630
> +  - qcom,mdss-dsi-ctrl-sdm660
> +  - qcom,mdss-dsi-ctrl-sdm845
> +  - qcom,mdss-dsi-ctrl-sm8250
> +  - const: qcom,mdss-dsi-ctrl

This is not explained in commit msg and looks like part of previous commit.


Best regards,
Krzysztof

Re: [PATCH v2 07/18] dt-bindings: msm: dsi-controller-main: Add compatible strings for every current SoC

On 08/11/2022 13:46, Dmitry Baryshkov wrote:
> On 08/11/2022 02:56, Bryan O'Donoghue wrote:
>> Currently we do not differentiate between the various users of the
>> qcom,mdss-dsi-ctrl. The driver is flexible enough to operate from one
>> compatible string but, the hardware does have some significant differences
>> in the number of clocks.
>>
>> To facilitate documenting the clocks add the following compatible strings
>>
>> - qcom,mdss-dsi-ctrl-apq8064
> 
> Generic comment: I think we'd better follow the arm/qcom-soc.yaml and 
> use qcom,soc-something as compat string. This would leave us with 
> qcom,apq8064-dsi-ctrl

That's a requirement. Only existing patterns are allowed to continue,
but here all there is no such pattern.

Best regards,
Krzysztof

Re: [PATCH v2 06/18] dt-bindings: msm: dsi-controller-main: Alphanumerically sort compatible enum

On 08/11/2022 00:56, Bryan O'Donoghue wrote:
> Sort the order of the compatible strings alphanumerically.
> 
> Cc: Rob Clark 
> Cc: Abhinav Kumar 
> Cc: Dmitry Baryshkov 
> Cc: Sean Paul 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: Rob Herring 
> Cc: Krzysztof Kozlowski 
> Cc: linux-arm-...@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: freedr...@lists.freedesktop.org
> Cc: devicet...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Signed-off-by: Bryan O'Donoghue 

I think here you can see how the commit msg looks now... One line of
message and 13 lines of CC.

With commit msg fixed:

Acked-by: Krzysztof Kozlowski 

Best regards,
Krzysztof

Re: [PATCH v2 05/18] dt-bindings: msm: dsi-controller-main: Fix description of core clock

On 08/11/2022 00:56, Bryan O'Donoghue wrote:
> There's a typo in describing the core clock as an 'escape' clock. The
> accurate description is 'core'.
> 
> Fixes: 4dbe55c97741 ("dt-bindings: msm: dsi: add yaml schemas for DSI 
> bindings")
> Cc: Rob Clark 


Acked-by: Krzysztof Kozlowski 

Best regards,
Krzysztof

Re: [PATCH v2 03/18] dt-bindings: msm: dsi-controller-main: Add vdd* descriptions back in

On 08/11/2022 00:56, Bryan O'Donoghue wrote:
> When converting from .txt to .yaml we didn't include descriptions for the
> existing regulator supplies.
> 
> - vdd
> - vdda
> - vddio
> 
> Add those descriptions into the yaml now as they were prior to the
> conversion. Mark the supplies as required as was previously the case in the
> .txt implementation.
> 
> Warnings about missing regulators can be resolved by updating the relevant
> dtsi files to point to fixed always-on regulators where appropriate.
> 
> Fixes: 4dbe55c97741 ("dt-bindings: msm: dsi: add yaml schemas for DSI 
> bindings")
> Cc: Rob Clark 
> Cc: Abhinav Kumar 
> Cc: Dmitry Baryshkov 
> Cc: Sean Paul 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: Rob Herring 
> Cc: Krzysztof Kozlowski 
> Cc: linux-arm-...@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: freedr...@lists.freedesktop.org
> Cc: devicet...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Signed-off-by: Bryan O'Donoghue 
> ---
>  .../bindings/display/msm/dsi-controller-main.yaml | 15 +++
>  1 file changed, 15 insertions(+)
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml 
> b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> index cf782c5f5bdb0..0f7747e55b9be 100644
> --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> @@ -124,6 +124,18 @@ properties:
>- port@0
>- port@1
>  
> +  vdd-supply:
> +description:
> +  Phandle to vdd regulator device node

Drop "Phandle to" and "device node", so just "VDD regulator". You
describe hardware rather (when applicable), not Devicetree syntax.

The same in other places and with that:


Reviewed-by: Krzysztof Kozlowski 

Best regards,
Krzysztof

Re: [PATCH v2 02/18] dt-bindings: msm: dsi-controller-main: Fix power-domain constraint

On 08/11/2022 00:56, Bryan O'Donoghue wrote:
> power-domain is required for the sc7180 dispcc GDSC but not every qcom SoC
> has a similar dependency for example the aqp8064.
> 
> Most Qcom SoC's using mdss-dsi-ctrl seem to have the ability to
> power-collapse the MDP without collapsing DSI.
> 
> For example the qcom vendor kernel commit for apq8084, msm8226, msm8916, 
> msm8974
> 
> https://review.carbonrom.org/plugins/gitiles/CarbonROM/android_kernel_oneplus_msm8994/+/7b5c011a770daa2811778937ed646237a28a8694
> 
> "ARM: dts: msm: add mdss gdsc supply to dsi controller device
> 
>  It is possible for the DSI controller to be active when MDP is
>  power collapsed. DSI controller needs to have it's own vote for
>  mdss gdsc to ensure that gdsc remains on in such cases."
> 
> This however doesn't appear to be the case for the apq8064 so we shouldn't
> be marking power-domain as required in yaml checks.
> 
> Fixes: 4dbe55c97741 ("dt-bindings: msm: dsi: add yaml schemas for DSI 
> bindings")
> Reviewed-by: Dmitry Baryshkov 
> Cc: Rob Clark 
> Cc: Abhinav Kumar 
> Cc: Dmitry Baryshkov 
> Cc: Sean Paul 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: Rob Herring 
> Cc: Krzysztof Kozlowski 
> Cc: linux-arm-...@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: freedr...@lists.freedesktop.org
> Cc: devicet...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org

Your Cc list is huge and not necessary to store in git log. For example
I am appearing there twice. Please keep it under '---'.

Acked-by: Krzysztof Kozlowski 

Best regards,
Krzysztof

Re: [PULL] nouveau-next

2022-11-08 Thread Sam Ravnborg

Hi Thomas.

>   drm/nouveau/kms: switch to drm fbdev helpers

Noticed this little gem between all the other goodies in this pull
request. Just in case it is interesting and you missed it.

Sam

Re: [Intel-gfx] [PATCH 1/2] drm/i915/gt: Add GT oriented dmesg output

2022-11-08 Thread John Harrison


On 11/8/2022 01:01, Tvrtko Ursulin wrote:

On 07/11/2022 19:14, John Harrison wrote:

On 11/7/2022 08:17, Tvrtko Ursulin wrote:

On 07/11/2022 09:33, Tvrtko Ursulin wrote:

On 05/11/2022 01:03, Ceraolo Spurio, Daniele wrote:

On 11/4/2022 10:25 AM, john.c.harri...@intel.com wrote:

From: John Harrison 

When trying to analyse bug reports from CI, customers, etc. it 
can be

difficult to work out exactly what is happening on which GT in a
multi-GT system. So add GT oriented debug/error message wrappers. If
used instead of the drm_ equivalents, you get the same output but 
with

a GT# prefix on it.

Signed-off-by: John Harrison 


The only downside to this is that we'll print "GT0: " even on 
single-GT devices. We could introduce a gt->info.name and print 
that, so we could have it different per-platform, but IMO it's not 
worth the effort.


Reviewed-by: Daniele Ceraolo Spurio 

I think it might be worth getting an ack from one of the 
maintainers to make sure we're all aligned on transitioning to 
these new logging macro for gt code.


Idea is I think a very good one. First I would suggest 
standardising to lowercase GT in logs because:


$ grep "GT%" i915/ -r
$ grep "gt%" i915/ -r
i915/gt/intel_gt_sysfs.c: gt->i915->sysfs_gt, "gt%d", gt->info.id))
i915/gt/intel_gt_sysfs.c:    "failed to initialize gt%d 
sysfs root\n", gt->info.id);
i915/gt/intel_gt_sysfs_pm.c: "failed to create 
gt%u RC6 sysfs files (%pe)\n",
i915/gt/intel_gt_sysfs_pm.c: "failed to create gt%u RC6p sysfs 
files (%pe)\n",
i915/gt/intel_gt_sysfs_pm.c: "failed to create 
gt%u RPS sysfs files (%pe)",
i915/gt/intel_gt_sysfs_pm.c: "failed to create 
gt%u punit_req_freq_mhz sysfs (%pe)",
i915/gt/intel_gt_sysfs_pm.c: "failed to create gt%u throttle sysfs 
files (%pe)",
i915/gt/intel_gt_sysfs_pm.c: "failed to create gt%u 
media_perf_power_attrs sysfs (%pe)\n",
i915/gt/intel_gt_sysfs_pm.c: "failed to add 
gt%u rps defaults (%pe)\n",
i915/i915_driver.c: drm_err(>->i915->drm, "gt%d: intel_pcode_init 
failed %d\n", id, ret);
i915/i915_hwmon.c:  snprintf(ddat_gt->name, 
sizeof(ddat_gt->name), "i915_gt%u", i);




Just because there are 11 existing instances of one form doesn't mean 
that the 275 instances that are waiting to be converted should be 
done incorrectly. GT is an acronym and should be capitalised.


Okay just make it consistent then.


Besides:
grep -r "GT " i915 | grep '"'
i915/vlv_suspend.c: drm_err(&i915->drm, "timeout 
disabling GT waking\n");
i915/vlv_suspend.c: "timeout waiting for GT wells 
to go %s\n",
i915/vlv_suspend.c: drm_dbg(&i915->drm, "GT register access while 
GT waking disabled\n");
i915/i915_gpu_error.c:  err_printf(m, "GT awake: %s\n", 
str_yes_no(gt->awake));

i915/i915_debugfs.c:    seq_printf(m, "GT awake? %s [%d], %llums\n",
i915/selftests/i915_gem_evict.c: pr_err("Failed to idle GT (on %s)", 
engine->name);
i915/intel_uncore.c:  "GT thread status wait timed 
out\n");
i915/gt/uc/selftest_guc_multi_lrc.c: drm_err(>->i915->drm, "GT 
failed to idle: %d\n", ret);
i915/gt/uc/selftest_guc.c: drm_err(>->i915->drm, "GT failed to 
idle: %d\n", ret);
i915/gt/uc/selftest_guc.c: drm_err(>->i915->drm, "GT failed to 
idle: %d\n", ret);
i915/gt/intel_gt_mcr.c: * Some GT registers are designed as 
"multicast" or "replicated" registers:
i915/gt/selftest_rps.c: pr_info("%s: rps counted %d 
C0 cycles [%lldns] in %lldns [%d cycles], using GT clock frequency of 
%uKHz\n",
i915/gt/selftest_hangcheck.c:   pr_err("[%s] GT is 
wedged!\n", engine->name);

i915/gt/selftest_hangcheck.c:   pr_err("GT is wedged!\n");
i915/gt/intel_gt_clock_utils.c: "GT clock frequency 
changed, was %uHz, now %uHz!\n",
i915/gt/selftest_engine_pm.c:   pr_err("Unable to flush GT pm 
before test\n");

i915/gt/selftest_engine_pm.c: pr_err("GT failed to idle\n");
i915/i915_sysfs.c:   "failed to register GT sysfs 
directory\n");
i915/intel_uncore.h: * of the basic non-engine GT registers 
(referred to as "GSI" on
i915/intel_uncore.h: * newer platforms, or "GT block" on older 
platforms)?  If so, we'll




Then there is a question of naming. Are we okay with GT_XXX or, do 
we want intel_gt_, or something completely different. I don't have 
a strong opinion at the moment so I'll add some more folks to Cc.


You mean GT_ERR("msg") vs intel_gt_err("msg")? Personally, I would 
prefer just gt_err("msg") to keep it as close to the official drm_* 
versions as possible. Print lines tend to be excessively long 
already. Taking a 'gt' parameter instead of a '>->i915->drm' 
parameter does help with that but it seems like calling the wrapper 
intel_gt_* is shooting ourselves in the foot on that one. And GT_ERR 
vs gt_err just comes down to the fact that it is a macro wrapper and 
therefore is required to be in upper case.


There was a mai

Re: [PATCH drm-misc-next v4 0/4] drm/arm/hdlcd: use drm managed resources

2022-11-08 Thread Danilo Krummrich


Hi Liviu,


The only issue that I'm seeing that is not critical is that at reboot/shutdown 
time
I'm getting an "Unexpected global fault, this could be serious" from the smmu:

[ 6893.467910] arm-smmu 7fb3.iommu: disabling translation
[ 6893.473550] ohci-platform 7ffb.usb: Removing from iommu group 1
[ 6893.479909] ehci-platform 7ffc.usb: Removing from iommu group 1
[ 6893.486931] arm-smmu 7fb1.iommu: disabling translation
[ 6893.492521] hdlcd 7ff5.hdlcd: Removing from iommu group 3
[ 6893.492650] arm-smmu 7fb1.iommu: Unexpected global fault, this could be 
serious
[ 6893.505959] arm-smmu 7fb1.iommu: GFSR 0x8001, GFSYNR0 
0x, GFSYNR1 0x, GFSYNR2 0x
[ 6893.516511] arm-smmu 7fb0.iommu: disabling translation
[ 6893.522195] dma-pl330 7ff0.dma-controller: Removing from iommu group 2
[ 6893.529607] arm-smmu 2b50.iommu: disabling translation
[ 6893.535221] pcieport :00:00.0: Removing from iommu group 0
[ 6893.541135] pci :01:00.0: Removing from iommu group 0
[ 6893.546604] pcieport :02:01.0: Removing from iommu group 0
[ 6893.552511] pcieport :02:02.0: Removing from iommu group 0
[ 6893.558418] pcieport :02:03.0: Removing from iommu group 0
[ 6893.564329] pcieport :02:0c.0: Removing from iommu group 0
[ 6893.570393] pcieport :02:10.0: Removing from iommu group 0
[ 6893.576314] pcieport :02:1f.0: Removing from iommu group 0
[ 6893.582214] sata_sil24 :03:00.0: Removing from iommu group 0
[ 6893.588270] sky2 :08:00.0: Removing from iommu group 0
[ 6893.594616] reboot: Power down


The reboot/shutdown succeeds, so I'm not too worried about it for now, but hope 
that
this is something you'll keep in mind in the later series when you do 
drm_dev_unplug().


Yes, I'd expect this to be related to the missing protection of platform 
device bound resources.




With that, for the whole series:

Acked-by: Liviu Dudau 

Thanks for the patience and going through the series iterations with me.

I can pull this series into drm-misc-next on Monday if you don't have any other 
plans.

Thanks, I saw you already applied the series.

Have you had a look on the same series for malidp?

- Danilo



Best regards,
Liviu

Re: [PATCH v2] drm/i915: Partial abandonment of legacy DRM logging macros

2022-11-08 Thread John Harrison


On 11/8/2022 04:26, Tvrtko Ursulin wrote:

From: Tvrtko Ursulin 

Convert some usages of legacy DRM logging macros into versions which tell
us on which device have the events occurred.

v2:
  * Don't have struct drm_device as local. (Jani, Ville)

Signed-off-by: Tvrtko Ursulin 
Cc: Jani Nikula 
Cc: John Harrison 
Cc: Ville Syrjälä 
---
  drivers/gpu/drm/i915/gem/i915_gem_context.c   |  2 +-
  .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 26 +++
  .../drm/i915/gt/intel_execlists_submission.c  | 13 +++---
  drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c  |  4 +-
  drivers/gpu/drm/i915/gt/intel_gt.c|  4 +-
  drivers/gpu/drm/i915/gt/intel_gt_irq.c|  8 ++--
  drivers/gpu/drm/i915/gt/intel_rps.c   |  6 ++-
  drivers/gpu/drm/i915/gt/intel_workarounds.c   | 43 +++
  .../gpu/drm/i915/gt/intel_workarounds_types.h |  4 ++
  .../gpu/drm/i915/gt/selftest_workarounds.c|  4 +-
  drivers/gpu/drm/i915/i915_debugfs.c   |  4 +-
  drivers/gpu/drm/i915/i915_gem.c   |  2 +-
  drivers/gpu/drm/i915/i915_getparam.c  |  2 +-
  drivers/gpu/drm/i915/i915_irq.c   | 12 +++---
  drivers/gpu/drm/i915/i915_perf.c  | 14 +++---
  drivers/gpu/drm/i915/i915_query.c | 12 +++---
  drivers/gpu/drm/i915/i915_sysfs.c |  3 +-
  drivers/gpu/drm/i915/i915_vma.c   | 16 ---
  drivers/gpu/drm/i915/intel_uncore.c   | 21 +
  19 files changed, 119 insertions(+), 81 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c 
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 01402f3c58f6..7f2831efc798 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -546,7 +546,7 @@ set_proto_ctx_engines_bond(struct i915_user_extension 
__user *base, void *data)
}
  
  	if (intel_engine_uses_guc(master)) {

-   DRM_DEBUG("bonding extension not supported with GuC 
submission");
+   drm_dbg(&i915->drm, "bonding extension not supported with GuC 
submission");
return -ENODEV;
}
  
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c

index 1160723c9d2d..f65fd03f7cf2 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -2148,7 +2148,8 @@ static int eb_move_to_gpu(struct i915_execbuffer *eb)
return err;
  }
  
-static int i915_gem_check_execbuffer(struct drm_i915_gem_execbuffer2 *exec)

+static int i915_gem_check_execbuffer(struct drm_i915_private *i915,
+struct drm_i915_gem_execbuffer2 *exec)
  {
if (exec->flags & __I915_EXEC_ILLEGAL_FLAGS)
return -EINVAL;
@@ -2161,7 +2162,7 @@ static int i915_gem_check_execbuffer(struct 
drm_i915_gem_execbuffer2 *exec)
}
  
  	if (exec->DR4 == 0x) {

-   DRM_DEBUG("UXA submitting garbage DR4, fixing up\n");
+   drm_dbg(&i915->drm, "UXA submitting garbage DR4, fixing up\n");
exec->DR4 = 0;
}
if (exec->DR1 || exec->DR4)
@@ -2799,7 +2800,8 @@ add_timeline_fence_array(struct i915_execbuffer *eb,
  
  		syncobj = drm_syncobj_find(eb->file, user_fence.handle);

if (!syncobj) {
-   DRM_DEBUG("Invalid syncobj handle provided\n");
+   drm_dbg(&eb->i915->drm,
+   "Invalid syncobj handle provided\n");
return -ENOENT;
}
  
@@ -2807,7 +2809,8 @@ add_timeline_fence_array(struct i915_execbuffer *eb,
  
  		if (!fence && user_fence.flags &&

!(user_fence.flags & I915_EXEC_FENCE_SIGNAL)) {
-   DRM_DEBUG("Syncobj handle has no fence\n");
+   drm_dbg(&eb->i915->drm,
+   "Syncobj handle has no fence\n");
drm_syncobj_put(syncobj);
return -EINVAL;
}
@@ -2816,7 +2819,9 @@ add_timeline_fence_array(struct i915_execbuffer *eb,
err = dma_fence_chain_find_seqno(&fence, point);
  
  		if (err && !(user_fence.flags & I915_EXEC_FENCE_SIGNAL)) {

-   DRM_DEBUG("Syncobj handle missing requested point 
%llu\n", point);
+   drm_dbg(&eb->i915->drm,
+   "Syncobj handle missing requested point %llu\n",
+   point);
dma_fence_put(fence);
drm_syncobj_put(syncobj);
return err;
@@ -2842,7 +2847,8 @@ add_timeline_fence_array(struct i915_execbuffer *eb,
 * 0) would break the timeline.
 */
if (user_fence.flags & I915_EXEC_FENCE_WAIT) {
-   DRM_DEBUG("Trying to wait & signal the same timeli

Re: [PATCH] drm/amd/display: Amend descriptions within enum pipe_split_policy

2022-11-08 Thread Harry Wentland

On 11/7/22 09:37, Carlos Bilbao wrote:
> Correct descriptions of two last fields of enum pipe_split_policy, updating
> comments with proper field names.
> 
> Signed-off-by: Carlos Bilbao 

Reviewed-by: Harry Wentland 

Harry

> ---
>  drivers/gpu/drm/amd/display/dc/dc.h | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dc.h 
> b/drivers/gpu/drm/amd/display/dc/dc.h
> index bfc5474c0f4c..277631a899d8 100644
> --- a/drivers/gpu/drm/amd/display/dc/dc.h
> +++ b/drivers/gpu/drm/amd/display/dc/dc.h
> @@ -457,15 +457,16 @@ enum pipe_split_policy {
>   MPC_SPLIT_DYNAMIC = 0,
>  
>   /**
> -  * @MPC_SPLIT_DYNAMIC: Avoid pipe split, which means that DC will not
> +  * @MPC_SPLIT_AVOID: Avoid pipe split, which means that DC will not
>* try any sort of split optimization.
>*/
>   MPC_SPLIT_AVOID = 1,
>  
>   /**
> -  * @MPC_SPLIT_DYNAMIC: With this option, DC will only try to optimize
> -  * the pipe utilization when using a single display; if the user
> -  * connects to a second display, DC will avoid pipe split.
> +  * @MPC_SPLIT_AVOID_MULT_DISP: With this option, DC will only try
> +  * to optimize the pipe utilization when using a single display;
> +  * if the user connects to a second display, DC will avoid pipe
> +  * split.
>*/
>   MPC_SPLIT_AVOID_MULT_DISP = 2,
>  };

Re: [PATCH v28 05/11] soc: mediatek: refine code to use mtk_mmsys_update_bits API

2022-11-08 Thread Nícolas F . R . A . Prado

On Tue, Nov 08, 2022 at 06:37:19PM +0100, Matthias Brugger wrote:
> 
> 
> On 07/11/2022 08:22, Nancy.Lin wrote:
> > Simplify code for update  mmsys reg.
> > 
> > Signed-off-by: Nancy.Lin 
> > Reviewed-by: AngeloGioacchino Del Regno 
> > 
> > Reviewed-by: CK Hu 
> > Tested-by: AngeloGioacchino Del Regno 
> > 
> > Tested-by: Bo-Chen Chen 
> > Reviewed-by: Nícolas F. R. A. Prado 
> > ---
> >   drivers/soc/mediatek/mtk-mmsys.c | 45 
> >   1 file changed, 16 insertions(+), 29 deletions(-)
> > 
> > diff --git a/drivers/soc/mediatek/mtk-mmsys.c 
> > b/drivers/soc/mediatek/mtk-mmsys.c
> > index 9a327eb5d9d7..73c8bd27e6ae 100644
> > --- a/drivers/soc/mediatek/mtk-mmsys.c
> > +++ b/drivers/soc/mediatek/mtk-mmsys.c
> > @@ -99,22 +99,27 @@ struct mtk_mmsys {
> > struct reset_controller_dev rcdev;
> >   };
> > +static void mtk_mmsys_update_bits(struct mtk_mmsys *mmsys, u32 offset, u32 
> > mask, u32 val)
> > +{
> > +   u32 tmp;
> > +
> > +   tmp = readl_relaxed(mmsys->regs + offset);
> > +   tmp = (tmp & ~mask) | (val & mask);
> 
> I'm not sure about the change in the implementation of
> mtk_mmsys_update_bits(). Nicolas tried to explain it to me on IRC but I
> wasn't totally convincing. As we have to go for at least another round of
> this patches, I'd like to get a clear understanding while it is needed that
> val bits are set to 1 in the mask.

The point here was to make sure that mtk_mmsys_update_bits() didn't allow
setting bits outside of the mask, since that's never what you want: the entire
point of having a mask is to specify the bits that should be updated (and the
ones that should be kept unchanged). So for example if you had

mask = 0x0ff0
val  = 0x00ff

the previous implementation would happily overwrite the 4 least significant bits
on the destination register, despite them not being present in the mask, which
is wrong.

This wrong behavior could easily lead to hard to trace bugs as soon as a badly
formatted/wrong val is passed and an unrelated bit updated due to the mask being
ignored.

For reference, _regmap_update_bits() does the same masking of the value [1].

That said, given that this function already existed and was just being moved,
it would've been cleaner to make this change in a separate commit.

[1] 
https://elixir.bootlin.com/linux/latest/source/drivers/base/regmap/regmap.c#L3122

Thanks,
Nícolas

> 
> Regards,
> Matthias
> 
> > +   writel_relaxed(tmp, mmsys->regs + offset);
> > +}
[..]
> > -static void mtk_mmsys_update_bits(struct mtk_mmsys *mmsys, u32 offset, u32 
> > mask, u32 val)
> > -{
> > -   u32 tmp;
> > -
> > -   tmp = readl_relaxed(mmsys->regs + offset);
> > -   tmp = (tmp & ~mask) | val;
> > -   writel_relaxed(tmp, mmsys->regs + offset);
> > -}
> > -
[..]

Re: [Intel-gfx] [PATCH] drm/i915: Don't wait forever in drop_caches

2022-11-08 Thread John Harrison


On 11/8/2022 01:08, Tvrtko Ursulin wrote:

On 07/11/2022 19:45, John Harrison wrote:

On 11/7/2022 06:09, Tvrtko Ursulin wrote:

On 04/11/2022 17:45, John Harrison wrote:

On 11/4/2022 03:01, Tvrtko Ursulin wrote:

On 03/11/2022 19:16, John Harrison wrote:

On 11/3/2022 02:38, Tvrtko Ursulin wrote:

On 03/11/2022 09:18, Tvrtko Ursulin wrote:

On 03/11/2022 01:33, John Harrison wrote:

On 11/2/2022 07:20, Tvrtko Ursulin wrote:

On 02/11/2022 12:12, Jani Nikula wrote:

On Tue, 01 Nov 2022, john.c.harri...@intel.com wrote:

From: John Harrison 

At the end of each test, IGT does a drop caches call via 
sysfs with


sysfs?
Sorry, that was meant to say debugfs. I've also been working 
on some sysfs IGT issues and evidently got my wires crossed!




special flags set. One of the possible paths waits for idle 
with an
infinite timeout. That causes problems for debugging issues 
when CI
catches a "can't go idle" test failure. Best case, the CI 
system times
out (after 90s), attempts a bunch of state dump actions and 
then
reboots the system to recover it. Worst case, the CI system 
can't do
anything at all and then times out (after 1000s) and simply 
reboots.
Sometimes a serial port log of dmesg might be available, 
sometimes not.


So rather than making life hard for ourselves, change the 
timeout to

be 10s rather than infinite. Also, trigger the standard
wedge/reset/recover sequence so that testing can continue 
with a

working system (if possible).

Signed-off-by: John Harrison 
---
  drivers/gpu/drm/i915/i915_debugfs.c | 7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c

index ae987e92251dd..9d916fbbfc27c 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -641,6 +641,9 @@ 
DEFINE_SIMPLE_ATTRIBUTE(i915_perf_noa_delay_fops,

    DROP_RESET_ACTIVE | \
    DROP_RESET_SEQNO | \
    DROP_RCU)
+
+#define DROP_IDLE_TIMEOUT    (HZ * 10)


I915_IDLE_ENGINES_TIMEOUT is defined in i915_drv.h. It's 
also only used

here.


So move here, dropping i915 prefix, next to the newly 
proposed one?

Sure, can do that.



I915_GEM_IDLE_TIMEOUT is defined in i915_gem.h. It's only 
used in

gt/intel_gt.c.


Move there and rename to GT_IDLE_TIMEOUT?

I915_GT_SUSPEND_IDLE_TIMEOUT is defined and used only in 
intel_gt_pm.c.


No action needed, maybe drop i915 prefix if wanted.

These two are totally unrelated and in code not being touched 
by this change. I would rather not conflate changing random 
other things with fixing this specific issue.



I915_IDLE_ENGINES_TIMEOUT is in ms, the rest are in jiffies.


Add _MS suffix if wanted.


My head spins.


I follow and raise that the newly proposed DROP_IDLE_TIMEOUT 
applies to DROP_ACTIVE and not only DROP_IDLE.
My original intention for the name was that is the 'drop 
caches timeout for intel_gt_wait_for_idle'. Which is quite the 
mouthful and hence abbreviated to DROP_IDLE_TIMEOUT. But yes, 
I realised later that name can be conflated with the DROP_IDLE 
flag. Will rename.





Things get refactored, code moves around, bits get left 
behind, who knows. No reason to get too worked up. :) As long 
as people are taking a wider view when touching the code 
base, and are not afraid to send cleanups, things should be 
good.
On the other hand, if every patch gets blocked in code review 
because someone points out some completely unrelated piece of 
code could be a bit better then nothing ever gets fixed. If 
you spot something that you think should be improved, isn't 
the general idea that you should post a patch yourself to 
improve it?


There's two maintainers per branch and an order of magnitude or 
two more developers so it'd be nice if cleanups would just be 
incoming on self-initiative basis. ;)


For the actual functional change at hand - it would be nice 
if code paths in question could handle SIGINT and then we 
could punt the decision on how long someone wants to wait 
purely to userspace. But it's probably hard and it's only 
debugfs so whatever.


The code paths in question will already abort on a signal 
won't they? Both intel_gt_wait_for_idle() and 
intel_guc_wait_for_pending_msg(), which is where the 
uc_wait_for_idle eventually ends up, have an 
'if(signal_pending) return -EINTR;' check. Beyond that, it 
sounds like what you are asking for is a change in the IGT 
libraries and/or CI framework to start sending signals after 
some specific timeout. That seems like a significantly more 
complex change (in terms of the number of entities affected 
and number of groups involved) and unnecessary.


If you say so, I haven't looked at them all. But if the code 
path in question already aborts on signals then I am not sure 
what is the patch fixing? I assumed you are trying to avoid the 
write stuck in D forever, which then prevents driver unload and 
everything, requiring the test runner to eventually reboot. If 
you say

Re: [PATCH v28 04/11] soc: mediatek: add mtk-mmsys support for mt8195 vdosys1

2022-11-08 Thread Nícolas F . R . A . Prado

On Tue, Nov 08, 2022 at 06:46:54PM +0100, Matthias Brugger wrote:
> On 07/11/2022 08:22, Nancy.Lin wrote:
[..]
> > --- a/drivers/soc/mediatek/mtk-mmsys.c
> > +++ b/drivers/soc/mediatek/mtk-mmsys.c
> > @@ -80,6 +80,12 @@ static const struct mtk_mmsys_driver_data 
> > mt8195_vdosys0_driver_data = {
> > .num_routes = ARRAY_SIZE(mmsys_mt8195_routing_table),
> >   };
> > +static const struct mtk_mmsys_driver_data mt8195_vdosys1_driver_data = {
> > +   .clk_driver = "clk-mt8195-vdo1",
> > +   .routes = mmsys_mt8195_vdo1_routing_table,
> > +   .num_routes = ARRAY_SIZE(mmsys_mt8195_vdo1_routing_table),
> > +};
> > +
> >   static const struct mtk_mmsys_driver_data mt8365_mmsys_driver_data = {
> > .clk_driver = "clk-mt8365-mm",
> > .routes = mt8365_mmsys_routing_table,
> > @@ -292,6 +298,10 @@ static const struct of_device_id of_match_mtk_mmsys[] 
> > = {
> > .compatible = "mediatek,mt8195-vdosys0",
> > .data = &mt8195_vdosys0_driver_data,
> 
> It seems we are missing a patch in the series. vdosys0 also correct was
> never introduced in the driver...

Hi Matthias,

as mentioned in the cover letter, this series is based on the series "Change
mmsys compatible for mt8195 mediatek-drm" [1], which introduces vdosys0. This
compatible entry specifically is added on patch 3 of that series [2].

[1] 
https://lore.kernel.org/all/20220927152704.12018-1-jason-jh@mediatek.com/
[2] 
https://lore.kernel.org/all/20220927152704.12018-4-jason-jh@mediatek.com/

Thanks,
Nícolas

> 
> > },
> > +   {
> > +   .compatible = "mediatek,mt8195-vdosys1",
> > +   .data = &mt8195_vdosys1_driver_data,
> > +   },
> > {
> > .compatible = "mediatek,mt8365-mmsys",
> > .data = &mt8365_mmsys_driver_data,

Re: [RFC PATCH 1/3] drm: Introduce color fill properties for drm plane

2022-11-08 Thread Simon Ser

cc'ing Pekka and wayland-devel for userspace devs feedback on the new uAPI.

On Saturday, October 29th, 2022 at 14:08, Dmitry Baryshkov 
 wrote:

> On 29/10/2022 01:59, Jessica Zhang wrote:
> > Add support for COLOR_FILL and COLOR_FILL_FORMAT properties for
> > drm_plane. In addition, add support for setting and getting the values
> > of these properties.
> >
> > COLOR_FILL represents the color fill of a plane while COLOR_FILL_FORMAT
> > represents the format of the color fill. Userspace can set enable solid
> > fill on a plane by assigning COLOR_FILL to a uint64_t value, assigning
> > the COLOR_FILL_FORMAT property to a uint32_t value, and setting the
> > framebuffer to NULL.
> >
> > Signed-off-by: Jessica Zhang 
> 
> Planes report supported formats using the drm_mode_getplane(). You'd
> also need to tell userspace, which formats are supported for color fill.
> I don't think one supports e.g. YV12.
> 
> A bit of generic comment for the discussion (this is an RFC anyway).
> Using color_fill/color_fill_format properties sounds simple, but this
> might be not generic enough. Limiting color_fill to 32 bits would
> prevent anybody from using floating point formats (e.g.
> DRM_FORMAT_XRGB16161616F, 64-bit value). Yes, this can be solved with
> e.g. using 64-bit for the color_fill value, but then this doesn't sound
> extensible too much.
> 
> So, a question for other hardware maintainers. Do we have hardware that
> supports such 'color filled' planes? Do we want to support format
> modifiers for filling color/data? Because what I have in mind is closer
> to the blob structure, which can then be used for filling the plane:
> 
> struct color_fill_blob {
>  u32 pixel_format;
>  u64 modifiers4];
>  u32 pixel_data_size; // fixme: is this necessary?
>  u8 pixel_data[];
> };
> 
> And then... This sounds a lot like a custom framebuffer.
> 
> So, maybe what should we do instead is to add new DRM_MODE_FB_COLOR_FILL
> flag to the framebuffers, which would e.g. mean that the FB gets stamped
> all over the plane. This would also save us from changing if (!fb)
> checks all over the drm core.
> 
> Another approach might be using a format modifier instead of the FB flag.
> 
> What do you think?

First off, we only need to represent the value of a single pixel here. So I'm
not quite following why we need format modifiers. Format modifiers describe how
pixels are laid out in memory. Since there's a single pixel described, this
is non-sensical to me, the format modifier is always LINEAR.

Then, I can understand why putting the pixel_format in there is tempting to
guarantee future extensibility, but it also adds complexity. For instance, how
does user-space figure out which formats can be used for COLOR_FILL? Can
user-space use any format supported by the plane? What does it mean for
multi-planar formats? Do we really want the kernel to have conversion logic for
all existing formats? Do we need to also add a new read-only blob prop to
indicate supported COLOR_FILL formats?

We've recently-ish standardized a new Wayland protocol [1] which has the same
purpose as this new kernel uAPI. The conclusion there was that using 32-bit
values for each channel (R, G, B, A) would be enough for almost all use-cases.
The driver can convert these high-precision values to what the hardware expects.
The only concern was about sending values outside of the [0.0, 1.0] range,
which may have HDR use-cases.

So, there are multiple ways to go about this. I can think of:

- Put "RGBA32" in the name of the prop, and if we ever need a different
  color format, pick a different name.
- Define a struct with an enum of possible fill kinds:
  #define FILL_COLOR_RGBA32 1
  #define FILL_COLOR_F32 2
  struct color_fill_blob { u32 kind; u8 data[]; };
- Define a struct with a version and RGBA values:
  struct color_fill_blob { u32 version; u32 rgba[4]; };
  If we need to add more formats later, or new metadata:
  struct color_fill_blob2 { u32 version; /* new fields */ };
  where version must be set to 2.
- Define a struct with a "pixel_format" prop, but force user-space to use a
  fixed format for now. Later, if we need another format, add a new prop to
  advertise supported formats.
- More complicated solutions, e.g. advertise the list of supported formats from
  the start.

[1]: 
https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/104

[PATCH v4] drm/sysfs: Link DRM connectors to corresponding Type-C connectors

2022-11-08 Thread Won Chung

Create a symlink pointing to USB Type-C connector for DRM connectors
when they are created. The link will be created only if the firmware is
able to describe the connection beween the two connectors.

Currently, even if a display uses a USB Type-C port, there is no way for
the userspace to find which port is used for which display. With the
symlink, display information would be accessible from Type-C connectors
and port information would be accessible from DRM connectors.
Associating the two subsystems, userspace would have potential to expose
and utilize more complex information, such as bandwidth used for a
specific USB Type-C port.

Signed-off-by: Won Chung 
Acked-by: Heikki Krogerus 
---
Changes from v3:
- Append to the commit message on why this patch is needed

Changes from v2:
- Resend the patch to dri-devel list

Changes from v1:
- Fix multiple lines to single line


 drivers/gpu/drm/drm_sysfs.c | 40 +
 1 file changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index 430e00b16eec..6a9904fa9186 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -11,12 +11,14 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -95,6 +97,34 @@ static char *drm_devnode(struct device *dev, umode_t *mode)
return kasprintf(GFP_KERNEL, "dri/%s", dev_name(dev));
 }
 
+static int typec_connector_bind(struct device *dev,
+   struct device *typec_connector, void *data)
+{
+   int ret;
+
+   ret = sysfs_create_link(&dev->kobj, &typec_connector->kobj, 
"typec_connector");
+   if (ret)
+   return ret;
+
+   ret = sysfs_create_link(&typec_connector->kobj, &dev->kobj, 
"drm_connector");
+   if (ret)
+   sysfs_remove_link(&dev->kobj, "typec_connector");
+
+   return ret;
+}
+
+static void typec_connector_unbind(struct device *dev,
+   struct device *typec_connector, void *data)
+{
+   sysfs_remove_link(&typec_connector->kobj, "drm_connector");
+   sysfs_remove_link(&dev->kobj, "typec_connector");
+}
+
+static const struct component_ops typec_connector_ops = {
+   .bind = typec_connector_bind,
+   .unbind = typec_connector_unbind,
+};
+
 static CLASS_ATTR_STRING(version, S_IRUGO, "drm 1.1.0 20060810");
 
 /**
@@ -355,6 +385,13 @@ int drm_sysfs_connector_add(struct drm_connector 
*connector)
if (connector->ddc)
return sysfs_create_link(&connector->kdev->kobj,
 &connector->ddc->dev.kobj, "ddc");
+
+   if (dev_fwnode(kdev)) {
+   r = component_add(kdev, &typec_connector_ops);
+   if (r)
+   drm_err(dev, "failed to add component\n");
+   }
+
return 0;
 
 err_free:
@@ -367,6 +404,9 @@ void drm_sysfs_connector_remove(struct drm_connector 
*connector)
if (!connector->kdev)
return;
 
+   if (dev_fwnode(connector->kdev))
+   component_del(connector->kdev, &typec_connector_ops);
+
if (connector->ddc)
sysfs_remove_link(&connector->kdev->kobj, "ddc");
 
-- 
2.37.3.998.g577e59143f-goog

[PATCH v2] drm: fix crash in drm_minor_alloc_release

2022-11-08 Thread Stanislaw Gruszka

If drm_sysfs_minor_alloc() fail in drm_minor_alloc() we can end up
freeing invalid minor->kdev pointer and drm_minor_alloc_release()
will crash like below:

RIP: 0010:kobject_put+0x19/0x1c0
RSP: 0018:bc7001637c38 EFLAGS: 00010282
RAX: a8d6deb0 RBX:  RCX: 9cb5912d4540
RDX: a9c45ec5 RSI: 9cb5902f2b68 RDI: fff4
RBP: fff4 R08: a9c40dec R09: 0008
R10: aa81f7d2 R11: aa81f7ca R12: 9cb5912d4540
R13: 9cb5912d4540 R14: dead0122 R15: dead0100
FS:  7f56b06e6740() GS:9cb728b4() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: 0030 CR3: 00011285b004 CR4: 00170ee0
DR0:  DR1:  DR2: 
DR3:  DR6: 07f0 DR7: 0400
Call Trace:
 
 drm_minor_alloc_release+0x19/0x50
 drm_managed_release+0xab/0x150
 drm_dev_init+0x21f/0x2f0
 __devm_drm_dev_alloc+0x3c/0xa0
 ivpu_probe+0x59/0x797 [intel_vpu 127058409b05eb2f99dcdecd3330bee28d6b3e76]
 pci_device_probe+0xa4/0x160
 really_probe+0x164/0x340
 __driver_probe_device+0x10d/0x190
 device_driver_attach+0x26/0x50
 bind_store+0x9f/0x120
 kernfs_fop_write_iter+0x12d/0x1c0
 new_sync_write+0x106/0x180
 vfs_write+0x216/0x2a0
 ksys_write+0x65/0xe0
 do_syscall_64+0x35/0x80
 entry_SYSCALL_64_after_hwframe+0x44/0xae

Fix this crash by returning NULL minor->kdev on error.

Fixes: f96306f9892b ("drm: manage drm_minor cleanup with drmm_")
Signed-off-by: Stanislaw Gruszka 
---
v2: return minor->kdev NULL pointer instead of checking for IS_ERR in
drm_minor_alloc_release()

 drivers/gpu/drm/drm_drv.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 8214a0b1ab7f..8d70b634d008 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -142,8 +142,11 @@ static int drm_minor_alloc(struct drm_device *dev, 
unsigned int type)
return r;
 
minor->kdev = drm_sysfs_minor_alloc(minor);
-   if (IS_ERR(minor->kdev))
-   return PTR_ERR(minor->kdev);
+   if (IS_ERR(minor->kdev)) {
+   r = PTR_ERR(minor->kdev);
+   minor->kdev = NULL;
+   return r;
+   }
 
*drm_minor_get_slot(dev, type) = minor;
return 0;
-- 
2.25.1

RFC - DRM-based UAPI for Dynamic Mux Switching

2022-11-08 Thread Daniel Dadap

Many dual-GPU notebook systems are equipped with a multiplexer ("mux")
to switch the signal source for the internal display panel between the
discrete and integrated GPUs. The vga-switcheroo infrastructure in the
Linux kernel can expose mux switch functionality to userspace via a
simple debugfs file; however, vga-switcheroo has several limitations
which make it unsuitable for some newer systems which support switching
the mux while the panel is actively driven (henceforth refered to as
"dynamic" mux switching), with the the intention to migrate display
responsibility from one GPU to the other. A new userspace API for mux
switching can overcome these limitations, while providing a cleaner
interface for DRM clients to interact with muxable displays.

Hardware which is capable of dynamic mux switching is now generally
available, labeled with the marketing name "NVIDIA Advanced Optimus":

https://www.nvidia.com/en-us/geforce/news/rtx-laptops-advanced-optimus/

Internally, we have been experimenting with implementing dynamic mux
switch capabilities on Linux for some time. An early X11/GLX prototype
was developed as internal proof of concept, but it relies on several
tricks to hide details about the mux switch from the X server and GLX
clients. A more recent prototype which extends vga-switcheroo and
updates the existing in-tree DRM-KMS drivers to use the new
vga-switcheroo extensions revealed the need for userspace to be more
actively involved in the mux switch process, leading to the design
proposal that follows:

Background: Limitations of vga-switcheroo

The current vga-switcheroo design has several limitations which are not
compatible with dynamic mux switching. The biggest of these is that the
system seems to be designed around the assumption that mux switches are
only possible when neither GPU is actively driving the display. The API
specifies a can_switch() callback function that "client" (i.e., GPU)
drivers register with vga-switcheroo so that they can report whether a
switch is possible. Currently, every implementation of this callback
will report that switching is possible IFF the reference count of active
modesetting clients is zero. There are currently three types of switch
events defined in the existing vga-switcheroo UAPI:

1. "Normal" mux switches (this switch type doesn't really have a name):
   Switch the mux immediately. This switch type calls the can_switch()
   callback and will fail if either client driver reports that it is
   busy and switching is not possible. If the client driver does not
   report that it handles power control, vga-switcheroo will power on
   the switched-to device and power off the switched-away-from device
   via the registered set_gpu_state() callbacks.
2. Delayed mux switches:
   Register the intent to switch the mux. The switch will be executed
   once there are no more active modesetting clients.
3. Mux-only switches:
   Switch the mux immediately, without checking the can_switch()
   callback, and without attempting to manipulate the power state of
   either GPU.

On the surface, the mux-only switch type seems like it might be suitable
for dynamic switches; however, this type of switch does not call *any*
of the client callbacks, so the client drivers are not made aware of the
mux switch event and cannot respond appropriately.

In particular, the reprobe() callback (which seems to be unimplemented
in every driver except for Nouveau) would be useful to alert the
switched-to GPU that it should reprobe the connected displays, e.g. to
retrain the link on eDP panels. Another callback that would be useful
for dynamic mux switching would be a new callback for enabling and
disabling Panel Self Refresh (PSR): this would be called on the
switched-away-from GPU to enable PSR before switching the mux, then
called on the switched-to GPU to disable PSR after switching the mux,
to enable a flicker-free transition from one GPU to the other.

vga-switcheroo is also built on the assumption that a system will have
at most one graphics mux, and that the mux is connected to exactly two
GPUs, one of them integrated and the other discrete. While this is the
most common configuration for GPU-muxed systems by far, there are some
existing designs with more than one mux, and it limits the possibile
configurations for future designs. Notably, existing systems with two
muxes (one for the internal panel, and another for an external display
connector) expose the muxes as a single logical mux which switches both
muxes at the same time.

These assumptions and limitations made sense for what was possible with
X.org at the time vga-switcheroo was designed: specifically, that there
can only ever be one GPU at a time with functioning 3D graphics, but in
this age of GLVND and PRIME Render Offloading, many multi-vendor use
cases are now possible which previously were not. While it is possible
to extend the existing vga-switcheroo subsystem to support dynamic
switching, and while vga-switcheroo could

Re: [PATCH v13,0/3] Add dpi output format control for MT8186


Hi Xinlei,

Somehow b4 broke with your thread but I was able to apply patch 1 and 2 by hand.

Thanks
Matthias

On 24/10/2022 04:04, xinlei@mediatek.com wrote:

From: Xinlei Lee 

Base on the branch of linus/master v6.1 rc1.

Change since v12:
1. Add MT8186_ prefix to variables added in mt8186-mmsys.h file.

Change since v11:
1. Rebase on v6.1-rc1. Change nothing.

Change since v10:
1. Modify patch title and add review tag.

Change since v9:
1. Modify the location of the mmsys_dev member variable.

Change since v8:
1. Modified the title and some description information.

Changes since v7:
1. This series is based on the following patch:
[1] soc: mediatek: Add mmsys func to adapt to dpi output for MT8186

https://patchwork.kernel.org/project/linux-mediatek/patch/1663161662-1598-2-git-send-email-xinlei@mediatek.com/
2. Modify the DPI_FORMAT_MASK macro definition to GENMASK(1, 0);
3. Add all settings to mtk_mmsys_ddp_dpi_fmt_config;
4. Modify the commit title to Add mt8186 dpi compatibles and platform
data.

Changes since v6:
1. Different from other ICs, when mt8186 DPI changes the output format,
the mmsys_base+400 register needs to be set to be valid at the same
time.
In this series, all the situations that mmsys need to be set up are
perfected (not necessarily used in practice).
2. Put the value that controls the mmsys function in mtk-mmsys.h.
3. Encountered the sink ic switched between dual edge and single edge,
perfected setting and clearing mmsys bit operations in mtk_dpi.c.

Changes since v5:
1. Separate the patch that adds edge_cfg_in_mmsys from the patch that
adds mt8186 dpi support.
2. Move the mmsys register definition to mmsys driver.
  
Changes since v4:

1. This series of cancellations is based on the following patches:
[1] Add MediaTek SoC(vdosys1) support for mt8195

https://patchwork.kernel.org/project/linux-mediatek/cover/20220711075245.10492-1-nancy@mediatek.com/
[2] Add MediaTek SoC DRM (vdosys1) support for mt8195

https://patchwork.kernel.org/project/linux-mediatek/cover/20220804072827.22383-1-nancy@mediatek.com/
2. Added mtk_mmsys_update_bits function in mtk-mmsys.c;
3. MMSYS 0x400 register is modified to MT8186_MMSYS_DPI_OUTPUT_FORMAT;
4. Fix formatting issues.

Changes since v3:
1. Fix formatting issues;
2. Modify the edge output control name & description;
3. Fix the threading problem.

Changes since v2:
1. Modify key nouns in the description;
2. Add the label of jitao to Co-developed-by;
3. Macro definition address lowercase problem and function naming;
4. Add missing a description of this property in the mtk_dpi_conf.

Change since v1:
1. Modify mt8186 compatiable location.
2. Modify MT8186_DPI_OUTPUT_FORMAT name.

When MT8186 outputs dpi signal, it is necessary to add dual edge output
format control in mmsys.

Xinlei Lee (3):
   soc: mediatek: Add all settings to mtk_mmsys_ddp_dpi_fmt_config func
   drm: mediatek: Set dpi format in mmsys
   drm: mediatek: Add mt8186 dpi compatibles and platform data

  drivers/gpu/drm/mediatek/mtk_dpi.c | 32 ++
  drivers/gpu/drm/mediatek/mtk_drm_drv.c |  2 ++
  drivers/soc/mediatek/mt8186-mmsys.h|  8 ---
  drivers/soc/mediatek/mtk-mmsys.c   | 27 +-
  include/linux/soc/mediatek/mtk-mmsys.h |  7 ++
  5 files changed, 67 insertions(+), 9 deletions(-)

Re: [RFC PATCH 1/3] drm: Introduce color fill properties for drm plane

2022-11-08 Thread Simon Ser

On Saturday, October 29th, 2022 at 13:23, Dmitry Baryshkov 
 wrote:

> On 29/10/2022 01:59, Jessica Zhang wrote:
> 
> > Add support for COLOR_FILL and COLOR_FILL_FORMAT properties for
> > drm_plane. In addition, add support for setting and getting the values
> > of these properties.
> > 
> > COLOR_FILL represents the color fill of a plane while COLOR_FILL_FORMAT
> > represents the format of the color fill. Userspace can set enable solid
> > fill on a plane by assigning COLOR_FILL to a uint64_t value, assigning
> > the COLOR_FILL_FORMAT property to a uint32_t value, and setting the
> > framebuffer to NULL.
> 
> I suppose that COLOR_FILL should override framebuffer rather than
> requiring that FB is set to NULL. In other words, if color_filL_format
> is non-zero, it would make sense to ignore the FB. Then one can use the
> color_fill_format property to quickly switch between filled plane and
> FB-backed one.

That would be inconsistent with the rest of the KMS uAPI. For instance,
the kernel will error out if CRTC has active=0 but a connector is still
linked to the CRTC. IOW, the current uAPI errors out if the KMS state
is inconsistent.

[Bug 203033] nouveau hung task

2022-11-08 Thread bugzilla-daemon

https://bugzilla.kernel.org/show_bug.cgi?id=203033

Murph (mu...@clurictec.com) changed:

   What|Removed |Added

 CC||mu...@clurictec.com

--- Comment #6 from Murph (mu...@clurictec.com) ---
Having a similar issue here. GUI locks up but system is still running in the
background (meetings can still see and hear me, I can hear them). Can't switch
to different tty, have to hard reset.

λ ~/ lspci | grep VGA
01:00.0 VGA compatible controller: NVIDIA Corporation TU117M [GeForce GTX 1650
Mobile / Max-Q] (rev a1)

λ ~/ uname -a
Linux murph-icl-gen2 5.15.74 #1-NixOS SMP Sat Oct 15 05:59:05 UTC 2022 x86_64
GNU/Linux

Nov 08 09:19:38 murph-icl-gen2 kernel: task:kworker/u32:42  state:D stack:0
pid:66054 ppid: 2 flags:0x4000
Nov 08 09:19:38 murph-icl-gen2 kernel: Workqueue: events_unbound
nv50_disp_atomic_commit_work [nouveau]
Nov 08 09:19:38 murph-icl-gen2 kernel: Call Trace:
Nov 08 09:19:38 murph-icl-gen2 kernel:  
Nov 08 09:19:38 murph-icl-gen2 kernel:  __schedule+0x2e1/0x1350
Nov 08 09:19:38 murph-icl-gen2 kernel:  ? nvkm_event_get+0x70/0x90 [nouveau]
Nov 08 09:19:38 murph-icl-gen2 kernel:  ? nvkm_client_notify_get+0x23/0x40
[nouveau]
Nov 08 09:19:38 murph-icl-gen2 kernel:  schedule+0x5b/0xd0
Nov 08 09:19:38 murph-icl-gen2 kernel:  schedule_timeout+0x104/0x140
Nov 08 09:19:38 murph-icl-gen2 kernel:  ?
nouveau_fence_enable_signaling+0x2a/0x70 [nouveau]
Nov 08 09:19:38 murph-icl-gen2 kernel:  dma_fence_default_wait+0x1a8/0x240
Nov 08 09:19:38 murph-icl-gen2 kernel:  ? dma_fence_free+0x20/0x20
Nov 08 09:19:38 murph-icl-gen2 kernel:  dma_fence_wait_timeout+0xb6/0xd0
Nov 08 09:19:38 murph-icl-gen2 kernel: 
drm_atomic_helper_wait_for_fences+0x82/0xe0 [drm_kms_helper]
Nov 08 09:19:38 murph-icl-gen2 kernel:  nv50_disp_atomic_commit_tail+0x90/0x850
[nouveau]
Nov 08 09:19:38 murph-icl-gen2 kernel:  process_one_work+0x1ee/0x390
Nov 08 09:19:38 murph-icl-gen2 kernel:  worker_thread+0x53/0x3e0
Nov 08 09:19:38 murph-icl-gen2 kernel:  ? process_one_work+0x390/0x390
Nov 08 09:19:38 murph-icl-gen2 kernel:  kthread+0x124/0x150
Nov 08 09:19:38 murph-icl-gen2 kernel:  ? set_kthread_struct+0x50/0x50
Nov 08 09:19:38 murph-icl-gen2 kernel:  ret_from_fork+0x1f/0x30
Nov 08 09:19:38 murph-icl-gen2 kernel:  

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

Re: [PATCH v13,1/3] soc: mediatek: Add all settings to mtk_mmsys_ddp_dpi_fmt_config func





On 08/11/2022 19:11, Matthias Brugger wrote:

Hi Xinlei,

Can you please fix the threading of this patches. I can see that this 1/3 but 
I'm not able to find 3/3 in the series. The first two look good, the third, I 
don't know.




Nevermind this message, the problem was on my side.

Regards,
Matthias


Regards,
Matthias

On 24/10/2022 04:04, xinlei@mediatek.com wrote:

From: Xinlei Lee 

The difference between MT8186 and other ICs is that when modifying the
output format, we need to modify the mmsys_base+0x400 register to take
effect. So when setting the dpi output format, we need to call
mtk_mmsys_ddp_dpi_fmt_config to set it to MT8186 synchronously.
Commit a071e52f75d1 ("soc: mediatek: Add mmsys func to adapt to dpi
output for MT8186") lacked some of the possible output formats and also
had a wrong bitmask.

Add the missing output formats and fix the bitmask.
While at it, also update mtk_mmsys_ddp_dpi_fmt_config() to use generic
formats, so that it is slightly easier to extend for other platforms.
Fixes: a071e52f75d1 ("soc: mediatek: Add mmsys func to adapt to dpi output for 
MT8186")

Signed-off-by: Xinlei Lee 
Reviewed-by: AngeloGioacchino Del Regno 

Reviewed-by: CK Hu 
Reviewed-by: Nícolas F. R. A. Prado 
---
  drivers/soc/mediatek/mt8186-mmsys.h    |  8 +---
  drivers/soc/mediatek/mtk-mmsys.c   | 27 --
  include/linux/soc/mediatek/mtk-mmsys.h |  7 +++
  3 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/drivers/soc/mediatek/mt8186-mmsys.h 
b/drivers/soc/mediatek/mt8186-mmsys.h

index 09b1ccbc0093..279d4138525b 100644
--- a/drivers/soc/mediatek/mt8186-mmsys.h
+++ b/drivers/soc/mediatek/mt8186-mmsys.h
@@ -5,9 +5,11 @@
  /* Values for DPI configuration in MMSYS address space */
  #define MT8186_MMSYS_DPI_OUTPUT_FORMAT    0x400
-#define DPI_FORMAT_MASK    0x1
-#define DPI_RGB888_DDR_CON    BIT(0)
-#define DPI_RGB565_SDR_CON    BIT(1)
+#define MT8186_DPI_FORMAT_MASK    GENMASK(1, 0)
+#define MT8186_DPI_RGB888_SDR_CON    0
+#define MT8186_DPI_RGB888_DDR_CON    1
+#define MT8186_DPI_RGB565_SDR_CON    2
+#define MT8186_DPI_RGB565_DDR_CON    3
  #define MT8186_MMSYS_OVL_CON    0xF04
  #define MT8186_MMSYS_OVL0_CON_MASK    0x3
diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
index 2e20b24da363..16cd924d8973 100644
--- a/drivers/soc/mediatek/mtk-mmsys.c
+++ b/drivers/soc/mediatek/mtk-mmsys.c
@@ -238,12 +238,27 @@ static void mtk_mmsys_update_bits(struct mtk_mmsys 
*mmsys, u32 offset, u32 mask,

  void mtk_mmsys_ddp_dpi_fmt_config(struct device *dev, u32 val)
  {
-    if (val)
-    mtk_mmsys_update_bits(dev_get_drvdata(dev), 
MT8186_MMSYS_DPI_OUTPUT_FORMAT,

-  DPI_RGB888_DDR_CON, DPI_FORMAT_MASK);
-    else
-    mtk_mmsys_update_bits(dev_get_drvdata(dev), 
MT8186_MMSYS_DPI_OUTPUT_FORMAT,

-  DPI_RGB565_SDR_CON, DPI_FORMAT_MASK);
+    struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
+
+    switch (val) {
+    case MTK_DPI_RGB888_SDR_CON:
+    mtk_mmsys_update_bits(mmsys, MT8186_MMSYS_DPI_OUTPUT_FORMAT,
+  MT8186_DPI_FORMAT_MASK, MT8186_DPI_RGB888_SDR_CON);
+    break;
+    case MTK_DPI_RGB565_SDR_CON:
+    mtk_mmsys_update_bits(mmsys, MT8186_MMSYS_DPI_OUTPUT_FORMAT,
+  MT8186_DPI_FORMAT_MASK, MT8186_DPI_RGB565_SDR_CON);
+    break;
+    case MTK_DPI_RGB565_DDR_CON:
+    mtk_mmsys_update_bits(mmsys, MT8186_MMSYS_DPI_OUTPUT_FORMAT,
+  MT8186_DPI_FORMAT_MASK, MT8186_DPI_RGB565_DDR_CON);
+    break;
+    case MTK_DPI_RGB888_DDR_CON:
+    default:
+    mtk_mmsys_update_bits(mmsys, MT8186_MMSYS_DPI_OUTPUT_FORMAT,
+  MT8186_DPI_FORMAT_MASK, MT8186_DPI_RGB888_DDR_CON);
+    break;
+    }
  }
  EXPORT_SYMBOL_GPL(mtk_mmsys_ddp_dpi_fmt_config);
diff --git a/include/linux/soc/mediatek/mtk-mmsys.h 
b/include/linux/soc/mediatek/mtk-mmsys.h

index d2b02bb43768..b85f66db33e1 100644
--- a/include/linux/soc/mediatek/mtk-mmsys.h
+++ b/include/linux/soc/mediatek/mtk-mmsys.h
@@ -9,6 +9,13 @@
  enum mtk_ddp_comp_id;
  struct device;
+enum mtk_dpi_out_format_con {
+    MTK_DPI_RGB888_SDR_CON,
+    MTK_DPI_RGB888_DDR_CON,
+    MTK_DPI_RGB565_SDR_CON,
+    MTK_DPI_RGB565_DDR_CON
+};
+
  enum mtk_ddp_comp_id {
  DDP_COMPONENT_AAL0,
  DDP_COMPONENT_AAL1,

Re: [PATCH v13,1/3] soc: mediatek: Add all settings to mtk_mmsys_ddp_dpi_fmt_config func


Hi Xinlei,

Can you please fix the threading of this patches. I can see that this 1/3 but 
I'm not able to find 3/3 in the series. The first two look good, the third, I 
don't know.


Regards,
Matthias

On 24/10/2022 04:04, xinlei@mediatek.com wrote:

From: Xinlei Lee 

The difference between MT8186 and other ICs is that when modifying the
output format, we need to modify the mmsys_base+0x400 register to take
effect. So when setting the dpi output format, we need to call
mtk_mmsys_ddp_dpi_fmt_config to set it to MT8186 synchronously.
   
Commit a071e52f75d1 ("soc: mediatek: Add mmsys func to adapt to dpi

output for MT8186") lacked some of the possible output formats and also
had a wrong bitmask.

Add the missing output formats and fix the bitmask.
   
While at it, also update mtk_mmsys_ddp_dpi_fmt_config() to use generic

formats, so that it is slightly easier to extend for other platforms.
   
Fixes: a071e52f75d1 ("soc: mediatek: Add mmsys func to adapt to dpi output for MT8186")

Signed-off-by: Xinlei Lee 
Reviewed-by: AngeloGioacchino Del Regno 

Reviewed-by: CK Hu 
Reviewed-by: Nícolas F. R. A. Prado 
---
  drivers/soc/mediatek/mt8186-mmsys.h|  8 +---
  drivers/soc/mediatek/mtk-mmsys.c   | 27 --
  include/linux/soc/mediatek/mtk-mmsys.h |  7 +++
  3 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/drivers/soc/mediatek/mt8186-mmsys.h 
b/drivers/soc/mediatek/mt8186-mmsys.h
index 09b1ccbc0093..279d4138525b 100644
--- a/drivers/soc/mediatek/mt8186-mmsys.h
+++ b/drivers/soc/mediatek/mt8186-mmsys.h
@@ -5,9 +5,11 @@
  
  /* Values for DPI configuration in MMSYS address space */

  #define MT8186_MMSYS_DPI_OUTPUT_FORMAT0x400
-#define DPI_FORMAT_MASK0x1
-#define DPI_RGB888_DDR_CON BIT(0)
-#define DPI_RGB565_SDR_CON BIT(1)
+#define MT8186_DPI_FORMAT_MASK GENMASK(1, 0)
+#define MT8186_DPI_RGB888_SDR_CON  0
+#define MT8186_DPI_RGB888_DDR_CON  1
+#define MT8186_DPI_RGB565_SDR_CON  2
+#define MT8186_DPI_RGB565_DDR_CON  3
  
  #define MT8186_MMSYS_OVL_CON			0xF04

  #define MT8186_MMSYS_OVL0_CON_MASK0x3
diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
index 2e20b24da363..16cd924d8973 100644
--- a/drivers/soc/mediatek/mtk-mmsys.c
+++ b/drivers/soc/mediatek/mtk-mmsys.c
@@ -238,12 +238,27 @@ static void mtk_mmsys_update_bits(struct mtk_mmsys 
*mmsys, u32 offset, u32 mask,
  
  void mtk_mmsys_ddp_dpi_fmt_config(struct device *dev, u32 val)

  {
-   if (val)
-   mtk_mmsys_update_bits(dev_get_drvdata(dev), 
MT8186_MMSYS_DPI_OUTPUT_FORMAT,
- DPI_RGB888_DDR_CON, DPI_FORMAT_MASK);
-   else
-   mtk_mmsys_update_bits(dev_get_drvdata(dev), 
MT8186_MMSYS_DPI_OUTPUT_FORMAT,
- DPI_RGB565_SDR_CON, DPI_FORMAT_MASK);
+   struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
+
+   switch (val) {
+   case MTK_DPI_RGB888_SDR_CON:
+   mtk_mmsys_update_bits(mmsys, MT8186_MMSYS_DPI_OUTPUT_FORMAT,
+ MT8186_DPI_FORMAT_MASK, 
MT8186_DPI_RGB888_SDR_CON);
+   break;
+   case MTK_DPI_RGB565_SDR_CON:
+   mtk_mmsys_update_bits(mmsys, MT8186_MMSYS_DPI_OUTPUT_FORMAT,
+ MT8186_DPI_FORMAT_MASK, 
MT8186_DPI_RGB565_SDR_CON);
+   break;
+   case MTK_DPI_RGB565_DDR_CON:
+   mtk_mmsys_update_bits(mmsys, MT8186_MMSYS_DPI_OUTPUT_FORMAT,
+ MT8186_DPI_FORMAT_MASK, 
MT8186_DPI_RGB565_DDR_CON);
+   break;
+   case MTK_DPI_RGB888_DDR_CON:
+   default:
+   mtk_mmsys_update_bits(mmsys, MT8186_MMSYS_DPI_OUTPUT_FORMAT,
+ MT8186_DPI_FORMAT_MASK, 
MT8186_DPI_RGB888_DDR_CON);
+   break;
+   }
  }
  EXPORT_SYMBOL_GPL(mtk_mmsys_ddp_dpi_fmt_config);
  
diff --git a/include/linux/soc/mediatek/mtk-mmsys.h b/include/linux/soc/mediatek/mtk-mmsys.h

index d2b02bb43768..b85f66db33e1 100644
--- a/include/linux/soc/mediatek/mtk-mmsys.h
+++ b/include/linux/soc/mediatek/mtk-mmsys.h
@@ -9,6 +9,13 @@
  enum mtk_ddp_comp_id;
  struct device;
  
+enum mtk_dpi_out_format_con {

+   MTK_DPI_RGB888_SDR_CON,
+   MTK_DPI_RGB888_DDR_CON,
+   MTK_DPI_RGB565_SDR_CON,
+   MTK_DPI_RGB565_DDR_CON
+};
+
  enum mtk_ddp_comp_id {
DDP_COMPONENT_AAL0,
DDP_COMPONENT_AAL1,

[PATCH v2 2/2] drm/v3d: add missing mutex_destroy

2022-11-08 Thread Maíra Canal

v3d_perfmon_open_file() instantiates a mutex for a particular file
instance, but it never destroys it by calling mutex_destroy() in
v3d_perfmon_close_file().

Similarly, v3d_perfmon_create_ioctl() instantiates a mutex for a
particular perfmon, but it never destroys it by calling mutex_destroy()
in v3d_perfmon_destroy_ioctl().

So, add the missing mutex_destroy on both cases.

Signed-off-by: Maíra Canal 
Reviewed-by: Daniel Vetter 
---
 drivers/gpu/drm/v3d/v3d_perfmon.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/v3d/v3d_perfmon.c 
b/drivers/gpu/drm/v3d/v3d_perfmon.c
index 48972c49..e1be7368b87d 100644
--- a/drivers/gpu/drm/v3d/v3d_perfmon.c
+++ b/drivers/gpu/drm/v3d/v3d_perfmon.c
@@ -17,8 +17,10 @@ void v3d_perfmon_get(struct v3d_perfmon *perfmon)
 
 void v3d_perfmon_put(struct v3d_perfmon *perfmon)
 {
-   if (perfmon && refcount_dec_and_test(&perfmon->refcnt))
+   if (perfmon && refcount_dec_and_test(&perfmon->refcnt)) {
+   mutex_destroy(&perfmon->lock);
kfree(perfmon);
+   }
 }
 
 void v3d_perfmon_start(struct v3d_dev *v3d, struct v3d_perfmon *perfmon)
@@ -113,6 +115,7 @@ void v3d_perfmon_close_file(struct v3d_file_priv *v3d_priv)
idr_for_each(&v3d_priv->perfmon.idr, v3d_perfmon_idr_del, NULL);
idr_destroy(&v3d_priv->perfmon.idr);
mutex_unlock(&v3d_priv->perfmon.lock);
+   mutex_destroy(&v3d_priv->perfmon.lock);
 }
 
 int v3d_perfmon_create_ioctl(struct drm_device *dev, void *data,
@@ -154,6 +157,7 @@ int v3d_perfmon_create_ioctl(struct drm_device *dev, void 
*data,
mutex_unlock(&v3d_priv->perfmon.lock);
 
if (ret < 0) {
+   mutex_destroy(&perfmon->lock);
kfree(perfmon);
return ret;
}
-- 
2.38.1

[PATCH v2 1/2] drm/v3d: switch to drmm_mutex_init

2022-11-08 Thread Maíra Canal

mutex_init is supposed to be balanced by a call to mutex_destroy, but
this is not currently happening on the v3d driver.

Considering the introduction of a DRM-managed mutex_init variant, switch
to the drmm_mutex_init.

Signed-off-by: Maíra Canal 
Reviewed-by: Daniel Vetter 
---
 drivers/gpu/drm/v3d/v3d_gem.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index b8980440d137..96af1cb5202a 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -10,6 +10,7 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 
@@ -1075,10 +1076,18 @@ v3d_gem_init(struct drm_device *dev)
 
spin_lock_init(&v3d->mm_lock);
spin_lock_init(&v3d->job_lock);
-   mutex_init(&v3d->bo_lock);
-   mutex_init(&v3d->reset_lock);
-   mutex_init(&v3d->sched_lock);
-   mutex_init(&v3d->cache_clean_lock);
+   ret = drmm_mutex_init(dev, &v3d->bo_lock);
+   if (ret)
+   return ret;
+   ret = drmm_mutex_init(dev, &v3d->reset_lock);
+   if (ret)
+   return ret;
+   ret = drmm_mutex_init(dev, &v3d->sched_lock);
+   if (ret)
+   return ret;
+   ret = drmm_mutex_init(dev, &v3d->cache_clean_lock);
+   if (ret)
+   return ret;
 
/* Note: We don't allocate address 0.  Various bits of HW
 * treat 0 as special, such as the occlusion query counters
-- 
2.38.1

[PATCH v2 0/2] Balance mutex_init and mutex_destroy calls

2022-11-08 Thread Maíra Canal

This series introduces some changes to assure the correct resource release on
the V3D driver, especially the mutex. Currently, the V3D has no mutex_destroy()
calls, which means that a mutex is being instantiated, but it is not being
released by the end of its use.

So, use the DRM-managed mutex_init variants when possible to manage the mutex
release and add mutex_destroy() calls when not possible.

Best Regards,
- Maíra Canal

v1 -> v2: 
https://lore.kernel.org/dri-devel/20221107224656.278135-1-mca...@igalia.com/T/#m3c23e3f21fe6aaae51138c746c74e94b2a7b3bfc

- Move mutex_destroy() to v3d_perfmon_put() (Daniel Vetter).
- Add mutex_destroy() on error case in v3d_perfmon_create_ioctl() (Daniel 
Vetter).
- Add Daniel Vetter's tags.

Maíra Canal (2):
  drm/v3d: switch to drmm_mutex_init
  drm/v3d: add missing mutex_destroy

 drivers/gpu/drm/v3d/v3d_gem.c | 17 +
 drivers/gpu/drm/v3d/v3d_perfmon.c |  6 +-
 2 files changed, 18 insertions(+), 5 deletions(-)

-- 
2.38.1

Re: [PATCH v2 07/11] vfio-iommufd: Support iommufd for physical VFIO devices

On Tue, Nov 08, 2022 at 03:41:25PM +0800, Yi Liu wrote:
> On 2022/11/8 14:10, Nicolin Chen wrote:
> > On Mon, Nov 07, 2022 at 08:52:51PM -0400, Jason Gunthorpe wrote:
> > 
> > > @@ -795,6 +800,10 @@ static int vfio_device_first_open(struct vfio_device 
> > > *device)
> > >   ret = vfio_group_use_container(device->group);
> > >   if (ret)
> > >   goto err_module_put;
> > > + } else if (device->group->iommufd) {
> > > + ret = vfio_iommufd_bind(device, device->group->iommufd);
> > 
> > Here we check device->group->iommufd...
> > 
> > > + if (ret)
> > > + goto err_module_put;
> > >   }
> > >   device->kvm = device->group->kvm;
> > > @@ -812,6 +821,7 @@ static int vfio_device_first_open(struct vfio_device 
> > > *device)
> > >   device->kvm = NULL;
> > >   if (device->group->container)
> > >   vfio_group_unuse_container(device->group);
> > > + vfio_iommufd_unbind(device);
> > 
> > ...yet, missing here, which could result in kernel oops.
> > 
> > Should probably add something similar:
> > +   if (device->group->iommufd)
> > +   vfio_iommufd_unbind(device);
> > 
> > Or should check !vdev->iommufd_device inside the ->unbind.
> 
> this check was in prior version, but removed in this version. any
> special reason? Jason?

Oooh, this makes more sense - Kevin pointed out the check was wrong:

> > +void vfio_iommufd_unbind(struct vfio_device *vdev)
> > +{
> > +   lockdep_assert_held(&vdev->dev_set->lock);
> > +
> > +   if (!vdev->iommufd_device)
> > +   return;

> there is no iommufd_device in the emulated path...

And he is right, so I dropped it. But really the check was just
misspelled, it was supposed to be "device->group->iommufd" because the
caller assumed it.

Still, I think the right way to fix it is to lift the check as we
don't touch group->iommufd in iommufd.c

Thanks,
Jason

Re: [PATCH v2 07/11] vfio-iommufd: Support iommufd for physical VFIO devices

On Mon, Nov 07, 2022 at 10:10:59PM -0800, Nicolin Chen wrote:

> > @@ -812,6 +821,7 @@ static int vfio_device_first_open(struct vfio_device 
> > *device)
> > device->kvm = NULL;
> > if (device->group->container)
> > vfio_group_unuse_container(device->group);
> > +   vfio_iommufd_unbind(device);
> 
> ...yet, missing here, which could result in kernel oops.
> 
> Should probably add something similar:
> + if (device->group->iommufd)
> + vfio_iommufd_unbind(device);
> 
> Or should check !vdev->iommufd_device inside the ->unbind.

Lets keep it symmetric since the container is checked:

@@ -821,7 +821,8 @@ static int vfio_device_first_open(struct vfio_device 
*device)
device->kvm = NULL;
if (device->group->container)
vfio_group_unuse_container(device->group);
-   vfio_iommufd_unbind(device);
+   else if (device->group->iommufd)
+   vfio_iommufd_unbind(device);
 err_module_put:
mutex_unlock(&device->group->group_lock);
module_put(device->dev->driver->owner);
@@ -840,7 +841,8 @@ static void vfio_device_last_close(struct vfio_device 
*device)
device->kvm = NULL;
if (device->group->container)
vfio_group_unuse_container(device->group);
-   vfio_iommufd_unbind(device);
+   else if (device->group->iommufd)
+   vfio_iommufd_unbind(device);
mutex_unlock(&device->group->group_lock);
module_put(device->dev->driver->owner);

Thanks,
Jason

Re: [PATCH v28 01/11] dt-bindings: arm: mediatek: mmsys: add vdosys1 compatible for MT8195





On 07/11/2022 08:22, Nancy.Lin wrote:

Add vdosys1 mmsys compatible for MT8195 platform.

For MT8195, VDOSYS0 and VDOSYS1 are 2 display HW pipelines binding to
2 different power domains, different clock drivers and different
mediatek-drm drivers.

Signed-off-by: Nancy.Lin 
Reviewed-by: Nícolas F. R. A. Prado 
---
  .../devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml  | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml 
b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
index 0711f1834fbd..aaabe2196185 100644
--- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
@@ -48,7 +48,9 @@ properties:
- const: syscon
  
- items:

-  - const: mediatek,mt8195-vdosys0
+  - enum:
+  - mediatek,mt8195-vdosys0
+  - mediatek,mt8195-vdosys1
- const: mediatek,mt8195-mmsys
- const: syscon
  


I think we had that several times already:
https://lore.kernel.org/all/6bbe9527-ae48-30e0-fb45-519223a74...@linaro.org/

We will something like this, but please check that this does not give any 
errors/warnings:


diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml 
b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml

index eb451bec23d3d..8e9c4f4d7c389 100644
--- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
@@ -32,13 +32,22 @@ properties:
   - mediatek,mt8183-mmsys
   - mediatek,mt8186-mmsys
   - mediatek,mt8192-mmsys
-  - mediatek,mt8195-mmsys
   - mediatek,mt8365-mmsys
   - const: syscon
   - items:
   - const: mediatek,mt7623-mmsys
   - const: mediatek,mt2701-mmsys
   - const: syscon
+  - items:
+  - const: mediatek,mt8195-vdosys0
+  - const: syscon
+  - items:
+  - const: mediatek,mt8195-vdosys1
+  - const: syscon
+  - items:
+  - const: mediatek,mt8195-mmsys
+  - const: syscon
+  deprecated: true

   reg:
 maxItems: 1

Re: [PATCH v28 04/11] soc: mediatek: add mtk-mmsys support for mt8195 vdosys1





On 07/11/2022 08:22, Nancy.Lin wrote:

Add mt8195 vdosys1 routing table to the driver data of mtk-mmsys.

Signed-off-by: Nancy.Lin 
Reviewed-by: AngeloGioacchino Del Regno 

Reviewed-by: Rex-BC Chen 
Reviewed-by: CK Hu 
Tested-by: AngeloGioacchino Del Regno 
Tested-by: Bo-Chen Chen 
---
  drivers/soc/mediatek/mt8195-mmsys.h | 139 
  drivers/soc/mediatek/mtk-mmsys.c|  10 ++
  2 files changed, 149 insertions(+)

diff --git a/drivers/soc/mediatek/mt8195-mmsys.h 
b/drivers/soc/mediatek/mt8195-mmsys.h
index abfe94a30248..fd7b455bd675 100644
--- a/drivers/soc/mediatek/mt8195-mmsys.h
+++ b/drivers/soc/mediatek/mt8195-mmsys.h
@@ -75,6 +75,70 @@
  #define MT8195_SOUT_DSC_WRAP1_OUT_TO_SINA_VIRTUAL0(2 << 16)
  #define MT8195_SOUT_DSC_WRAP1_OUT_TO_VPP_MERGE(3 << 
16)
  
+#define MT8195_VDO1_VPP_MERGE0_P0_SEL_IN			0xf04

+#define MT8195_VPP_MERGE0_P0_SEL_IN_FROM_MDP_RDMA0 1
+
+#define MT8195_VDO1_VPP_MERGE0_P1_SEL_IN   0xf08
+#define MT8195_VPP_MERGE0_P1_SEL_IN_FROM_MDP_RDMA1 1
+
+#define MT8195_VDO1_DISP_DPI1_SEL_IN   0xf10
+#define MT8195_DISP_DPI1_SEL_IN_FROM_VPP_MERGE4_MOUT   0
+
+#define MT8195_VDO1_DISP_DP_INTF0_SEL_IN   0xf14
+#define MT8195_DISP_DP_INTF0_SEL_IN_FROM_VPP_MERGE4_MOUT   0
+
+#define MT8195_VDO1_MERGE4_SOUT_SEL0xf18
+#define MT8195_MERGE4_SOUT_TO_DPI1_SEL 2
+#define MT8195_MERGE4_SOUT_TO_DP_INTF0_SEL 3
+
+#define MT8195_VDO1_MIXER_IN1_SEL_IN   0xf24
+#define MT8195_MIXER_IN1_SEL_IN_FROM_MERGE0_ASYNC_SOUT 1
+
+#define MT8195_VDO1_MIXER_IN2_SEL_IN   0xf28
+#define MT8195_MIXER_IN2_SEL_IN_FROM_MERGE1_ASYNC_SOUT 1
+
+#define MT8195_VDO1_MIXER_IN3_SEL_IN   0xf2c
+#define MT8195_MIXER_IN3_SEL_IN_FROM_MERGE2_ASYNC_SOUT 1
+
+#define MT8195_VDO1_MIXER_IN4_SEL_IN   0xf30
+#define MT8195_MIXER_IN4_SEL_IN_FROM_MERGE3_ASYNC_SOUT 1
+
+#define MT8195_VDO1_MIXER_OUT_SOUT_SEL 0xf34
+#define MT8195_MIXER_SOUT_TO_MERGE4_ASYNC_SEL  1
+
+#define MT8195_VDO1_VPP_MERGE1_P0_SEL_IN   0xf3c
+#define MT8195_VPP_MERGE1_P0_SEL_IN_FROM_MDP_RDMA2 1
+
+#define MT8195_VDO1_MERGE0_ASYNC_SOUT_SEL  0xf40
+#define MT8195_SOUT_TO_MIXER_IN1_SEL   1
+
+#define MT8195_VDO1_MERGE1_ASYNC_SOUT_SEL  0xf44
+#define MT8195_SOUT_TO_MIXER_IN2_SEL   1
+
+#define MT8195_VDO1_MERGE2_ASYNC_SOUT_SEL  0xf48
+#define MT8195_SOUT_TO_MIXER_IN3_SEL   1
+
+#define MT8195_VDO1_MERGE3_ASYNC_SOUT_SEL  0xf4c
+#define MT8195_SOUT_TO_MIXER_IN4_SEL   1
+
+#define MT8195_VDO1_MERGE4_ASYNC_SEL_IN0xf50
+#define MT8195_MERGE4_ASYNC_SEL_IN_FROM_MIXER_OUT_SOUT 1
+
+#define MT8195_VDO1_MIXER_IN1_SOUT_SEL 0xf58
+#define MT8195_MIXER_IN1_SOUT_TO_DISP_MIXER0
+
+#define MT8195_VDO1_MIXER_IN2_SOUT_SEL 0xf5c
+#define MT8195_MIXER_IN2_SOUT_TO_DISP_MIXER0
+
+#define MT8195_VDO1_MIXER_IN3_SOUT_SEL 0xf60
+#define MT8195_MIXER_IN3_SOUT_TO_DISP_MIXER0
+
+#define MT8195_VDO1_MIXER_IN4_SOUT_SEL 0xf64
+#define MT8195_MIXER_IN4_SOUT_TO_DISP_MIXER0
+
+#define MT8195_VDO1_MIXER_SOUT_SEL_IN  0xf68
+#define MT8195_MIXER_SOUT_SEL_IN_FROM_DISP_MIXER   0
+
  static const struct mtk_mmsys_routes mmsys_mt8195_routing_table[] = {
{
DDP_COMPONENT_OVL0, DDP_COMPONENT_RDMA0,
@@ -367,4 +431,79 @@ static const struct mtk_mmsys_routes 
mmsys_mt8195_routing_table[] = {
}
  };
  
+static const struct mtk_mmsys_routes mmsys_mt8195_vdo1_routing_table[] = {

+   {
+   DDP_COMPONENT_MDP_RDMA0, DDP_COMPONENT_MERGE1,
+   MT8195_VDO1_VPP_MERGE0_P0_SEL_IN, GENMASK(0, 0),
+   MT8195_VPP_MERGE0_P0_SEL_IN_FROM_MDP_RDMA0
+   }, {
+   DDP_COMPONENT_MDP_RDMA1, DDP_COMPONENT_MERGE1,
+   MT8195_VDO1_VPP_MERGE0_P1_SEL_IN, GENMASK(0, 0),
+   MT8195_VPP_MERGE0_P1_SEL_IN_FROM_MDP_RDMA1
+   }, {
+   DDP_COMPONENT_MDP_RDMA2, DDP_COMPONENT_MERGE2,
+   MT8195_VDO1_VPP_MERGE1_P0_SEL_IN, GENMASK(0, 0),
+   MT8195_VPP_MERGE1_P0_SEL_IN_FROM_MDP_RDMA2
+   }, {
+   DDP_COMPONENT_MERGE1, DDP_COMPONENT_ETHDR_MIXER,
+   MT8195_VDO1_

Re: [PATCH v28 06/11] soc: mediatek: add mtk-mmsys config API for mt8195 vdosys1





On 07/11/2022 08:22, Nancy.Lin wrote:

Add four mmsys config APIs. The config APIs are used for config
mmsys reg. Some mmsys regs need to be set according to the
HW engine binding to the mmsys simultaneously.

1. mtk_mmsys_merge_async_config: config merge async width/height.
async is used for cross-clock domain synchronization.
2. mtk_mmsys_hdr_confing: config hdr backend async width/height.
3. mtk_mmsys_mixer_in_config and mtk_mmsys_mixer_in_config:
config mixer related settings.

Signed-off-by: Nancy.Lin 
Reviewed-by: AngeloGioacchino Del Regno 

Reviewed-by: CK Hu 
Tested-by: AngeloGioacchino Del Regno 
Tested-by: Bo-Chen Chen 


Not something we need to fix in this series, but it would make sense instead of 
adding all the EXPORTS to pass the functions as callbacks in the 
platform_device_register_data. But I realize you don't pass the VDOSYS number to 
the DRM driver to distinguish between the different MMSYS devices that created 
the platform device. I hadn't had a deep look on the DRM implementation but I 
suppose it will be challenge...


Regards,
Matthias


---
  drivers/soc/mediatek/mt8195-mmsys.h|  6 +
  drivers/soc/mediatek/mtk-mmsys.c   | 35 ++
  include/linux/soc/mediatek/mtk-mmsys.h |  9 +++
  3 files changed, 50 insertions(+)

diff --git a/drivers/soc/mediatek/mt8195-mmsys.h 
b/drivers/soc/mediatek/mt8195-mmsys.h
index fd7b455bd675..454944a9409c 100644
--- a/drivers/soc/mediatek/mt8195-mmsys.h
+++ b/drivers/soc/mediatek/mt8195-mmsys.h
@@ -75,6 +75,12 @@
  #define MT8195_SOUT_DSC_WRAP1_OUT_TO_SINA_VIRTUAL0(2 << 16)
  #define MT8195_SOUT_DSC_WRAP1_OUT_TO_VPP_MERGE(3 << 
16)
  
+#define MT8195_VDO1_MERGE0_ASYNC_CFG_WD0xe30

+#define MT8195_VDO1_HDRBE_ASYNC_CFG_WD 0xe70
+#define MT8195_VDO1_HDR_TOP_CFG0xd00
+#define MT8195_VDO1_MIXER_IN1_ALPHA0xd30
+#define MT8195_VDO1_MIXER_IN1_PAD  0xd40
+
  #define MT8195_VDO1_VPP_MERGE0_P0_SEL_IN  0xf04
  #define MT8195_VPP_MERGE0_P0_SEL_IN_FROM_MDP_RDMA01
  
diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c

index 73c8bd27e6ae..6040a3cff6f8 100644
--- a/drivers/soc/mediatek/mtk-mmsys.c
+++ b/drivers/soc/mediatek/mtk-mmsys.c
@@ -137,6 +137,41 @@ void mtk_mmsys_ddp_disconnect(struct device *dev,
  }
  EXPORT_SYMBOL_GPL(mtk_mmsys_ddp_disconnect);
  
+void mtk_mmsys_merge_async_config(struct device *dev, int idx, int width, int height)

+{
+   mtk_mmsys_update_bits(dev_get_drvdata(dev), 
MT8195_VDO1_MERGE0_ASYNC_CFG_WD + 0x10 * idx,
+ ~0, height << 16 | width);
+}
+EXPORT_SYMBOL_GPL(mtk_mmsys_merge_async_config);
+
+void mtk_mmsys_hdr_config(struct device *dev, int be_width, int be_height)
+{
+   mtk_mmsys_update_bits(dev_get_drvdata(dev), 
MT8195_VDO1_HDRBE_ASYNC_CFG_WD, ~0,
+ be_height << 16 | be_width);
+}
+EXPORT_SYMBOL_GPL(mtk_mmsys_hdr_config);
+
+void mtk_mmsys_mixer_in_config(struct device *dev, int idx, bool alpha_sel, 
u16 alpha,
+  u8 mode, u32 biwidth)
+{
+   struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
+
+   mtk_mmsys_update_bits(mmsys, MT8195_VDO1_MIXER_IN1_ALPHA + (idx - 1) * 
4, ~0,
+ alpha << 16 | alpha);
+   mtk_mmsys_update_bits(mmsys, MT8195_VDO1_HDR_TOP_CFG, BIT(19 + idx),
+ alpha_sel << (19 + idx));
+   mtk_mmsys_update_bits(mmsys, MT8195_VDO1_MIXER_IN1_PAD + (idx - 1) * 4,
+ GENMASK(31, 16) | GENMASK(1, 0), biwidth << 16 | 
mode);
+}
+EXPORT_SYMBOL_GPL(mtk_mmsys_mixer_in_config);
+
+void mtk_mmsys_mixer_in_channel_swap(struct device *dev, int idx, bool 
channel_swap)
+{
+   mtk_mmsys_update_bits(dev_get_drvdata(dev), MT8195_VDO1_MIXER_IN1_PAD + 
(idx - 1) * 4,
+ BIT(4), channel_swap << 4);
+}
+EXPORT_SYMBOL_GPL(mtk_mmsys_mixer_in_channel_swap);
+
  void mtk_mmsys_ddp_dpi_fmt_config(struct device *dev, u32 val)
  {
if (val)
diff --git a/include/linux/soc/mediatek/mtk-mmsys.h 
b/include/linux/soc/mediatek/mtk-mmsys.h
index 127f1b888ace..a4708859c188 100644
--- a/include/linux/soc/mediatek/mtk-mmsys.h
+++ b/include/linux/soc/mediatek/mtk-mmsys.h
@@ -75,4 +75,13 @@ void mtk_mmsys_ddp_disconnect(struct device *dev,
  
  void mtk_mmsys_ddp_dpi_fmt_config(struct device *dev, u32 val);
  
+void mtk_mmsys_merge_async_config(struct device *dev, int idx, int width, int height);

+
+void mtk_mmsys_hdr_config(struct device *dev, int be_width, int be_height);
+
+void mtk_mmsys_mixer_in_config(struct device *dev, int idx, bool alpha_sel, 
u16 alpha,
+  u8 mode, u32 biwidth);
+
+void mtk_mmsys_mixer_in_channel_swap(struct device *dev, int idx, bool 
channel_swap);
+
  #endif /* __MTK_MMSYS_H */

Re: [PATCH] drm/panfrost: Split io-pgtable requests properly

2022-11-08 Thread Dmitry Osipenko

On 11/8/22 20:06, Robin Murphy wrote:
> Although we don't use 1GB block mappings, we still need to split
> map/unmap requests at 1GB boundaries to match what io-pgtable expects.
> Fix that, and add some explanation to make sense of it all.
> 
> Fixes: 3740b081795a ("drm/panfrost: Update io-pgtable API")
> Reported-by: Dmitry Osipenko 
> Signed-off-by: Robin Murphy 
> ---
> The previous diff turned out to be not quite right, so I've not
> included Dmitry's Tested-by given for that.
> ---
>  drivers/gpu/drm/panfrost/panfrost_mmu.c | 11 ++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c 
> b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> index e246d914e7f6..4e83a1891f3e 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> @@ -250,13 +250,22 @@ void panfrost_mmu_reset(struct panfrost_device *pfdev)
>  
>  static size_t get_pgsize(u64 addr, size_t size, size_t *count)
>  {
> + /*
> +  * io-pgtable only operates on multiple pages within a single table
> +  * entry, so we need to split at boundaries of the table size, i.e.
> +  * the next block size up. The distance from address A to the next
> +  * boundary of block size B is logically B - A % B, but in unsigned
> +  * two's complement where B is a power of two we get the equivalence
> +  * B - A % B == (B - A) % B == (n * B - A) % B, and choose n = 0 :)
> +  */
>   size_t blk_offset = -addr % SZ_2M;
>  
>   if (blk_offset || size < SZ_2M) {
>   *count = min_not_zero(blk_offset, size) / SZ_4K;
>   return SZ_4K;
>   }
> - *count = size / SZ_2M;
> + blk_offset = -addr % SZ_1G ?: SZ_1G;
> + *count = min(blk_offset, size) / SZ_2M;
>   return SZ_2M;
>  }
>  

This variant also works fine, thanks!

Tested-by: Dmitry Osipenko 

-- 
Best regards,
Dmitry

Re: [PATCH v28 05/11] soc: mediatek: refine code to use mtk_mmsys_update_bits API





On 07/11/2022 08:22, Nancy.Lin wrote:

Simplify code for update  mmsys reg.

Signed-off-by: Nancy.Lin 
Reviewed-by: AngeloGioacchino Del Regno 

Reviewed-by: CK Hu 
Tested-by: AngeloGioacchino Del Regno 
Tested-by: Bo-Chen Chen 
Reviewed-by: Nícolas F. R. A. Prado 
---
  drivers/soc/mediatek/mtk-mmsys.c | 45 
  1 file changed, 16 insertions(+), 29 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
index 9a327eb5d9d7..73c8bd27e6ae 100644
--- a/drivers/soc/mediatek/mtk-mmsys.c
+++ b/drivers/soc/mediatek/mtk-mmsys.c
@@ -99,22 +99,27 @@ struct mtk_mmsys {
struct reset_controller_dev rcdev;
  };
  
+static void mtk_mmsys_update_bits(struct mtk_mmsys *mmsys, u32 offset, u32 mask, u32 val)

+{
+   u32 tmp;
+
+   tmp = readl_relaxed(mmsys->regs + offset);
+   tmp = (tmp & ~mask) | (val & mask);


I'm not sure about the change in the implementation of mtk_mmsys_update_bits(). 
Nicolas tried to explain it to me on IRC but I wasn't totally convincing. As we 
have to go for at least another round of this patches, I'd like to get a clear 
understanding while it is needed that val bits are set to 1 in the mask.


Regards,
Matthias


+   writel_relaxed(tmp, mmsys->regs + offset);
+}
+
  void mtk_mmsys_ddp_connect(struct device *dev,
   enum mtk_ddp_comp_id cur,
   enum mtk_ddp_comp_id next)
  {
struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
const struct mtk_mmsys_routes *routes = mmsys->data->routes;
-   u32 reg;
int i;
  
  	for (i = 0; i < mmsys->data->num_routes; i++)

-   if (cur == routes[i].from_comp && next == routes[i].to_comp) {
-   reg = readl_relaxed(mmsys->regs + routes[i].addr);
-   reg &= ~routes[i].mask;
-   reg |= routes[i].val;
-   writel_relaxed(reg, mmsys->regs + routes[i].addr);
-   }
+   if (cur == routes[i].from_comp && next == routes[i].to_comp)
+   mtk_mmsys_update_bits(mmsys, routes[i].addr, 
routes[i].mask,
+ routes[i].val);
  }
  EXPORT_SYMBOL_GPL(mtk_mmsys_ddp_connect);
  
@@ -124,27 +129,14 @@ void mtk_mmsys_ddp_disconnect(struct device *dev,

  {
struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
const struct mtk_mmsys_routes *routes = mmsys->data->routes;
-   u32 reg;
int i;
  
  	for (i = 0; i < mmsys->data->num_routes; i++)

-   if (cur == routes[i].from_comp && next == routes[i].to_comp) {
-   reg = readl_relaxed(mmsys->regs + routes[i].addr);
-   reg &= ~routes[i].mask;
-   writel_relaxed(reg, mmsys->regs + routes[i].addr);
-   }
+   if (cur == routes[i].from_comp && next == routes[i].to_comp)
+   mtk_mmsys_update_bits(mmsys, routes[i].addr, 
routes[i].mask, 0);
  }
  EXPORT_SYMBOL_GPL(mtk_mmsys_ddp_disconnect);
  
-static void mtk_mmsys_update_bits(struct mtk_mmsys *mmsys, u32 offset, u32 mask, u32 val)

-{
-   u32 tmp;
-
-   tmp = readl_relaxed(mmsys->regs + offset);
-   tmp = (tmp & ~mask) | val;
-   writel_relaxed(tmp, mmsys->regs + offset);
-}
-
  void mtk_mmsys_ddp_dpi_fmt_config(struct device *dev, u32 val)
  {
if (val)
@@ -161,18 +153,13 @@ static int mtk_mmsys_reset_update(struct 
reset_controller_dev *rcdev, unsigned l
  {
struct mtk_mmsys *mmsys = container_of(rcdev, struct mtk_mmsys, rcdev);
unsigned long flags;
-   u32 reg;
  
  	spin_lock_irqsave(&mmsys->lock, flags);
  
-	reg = readl_relaxed(mmsys->regs + mmsys->data->sw0_rst_offset);

-
if (assert)
-   reg &= ~BIT(id);
+   mtk_mmsys_update_bits(mmsys, mmsys->data->sw0_rst_offset, 
BIT(id), 0);
else
-   reg |= BIT(id);
-
-   writel_relaxed(reg, mmsys->regs + mmsys->data->sw0_rst_offset);
+   mtk_mmsys_update_bits(mmsys, mmsys->data->sw0_rst_offset, 
BIT(id), BIT(id));
  
  	spin_unlock_irqrestore(&mmsys->lock, flags);

[PATCH v4 4/4] drm: panel: Add Jadard JD9365DA-H3 DSI panel

Jadard JD9365DA-H3 is WXGA MIPI DSI panel and it support TFT
dot matrix LCD with 800RGBx1280 dots at maximum.

Add support for it.

Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Jagan Teki 
---
Changes for v4:
- add delay explictly
- update init sequence
Changes for v3:
- updatd to WXGA
- use JD9365DA_CMD_DCS and JD9365DA_CMD_DELAY
Changes for v2:
- none

 MAINTAINERS   |   1 +
 drivers/gpu/drm/panel/Kconfig |  10 +
 drivers/gpu/drm/panel/Makefile|   1 +
 .../gpu/drm/panel/panel-jadard-jd9365da-h3.c  | 473 ++
 4 files changed, 485 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 8cd909cd34e0..5f66378dcfb0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6534,6 +6534,7 @@ DRM DRIVER FOR JADARD JD9365DA-H3 MIPI-DSI LCD PANELS
 M: Jagan Teki 
 S: Maintained
 F: Documentation/devicetree/bindings/display/panel/jadard,jd9365da-h3.yaml
+F: drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
 
 DRM DRIVER FOR LOGICVC DISPLAY CONTROLLER
 M: Paul Kocialkowski 
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index a582ddd583c2..6f6cc6662b2e 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -203,6 +203,16 @@ config DRM_PANEL_INNOLUX_P079ZCA
  24 bit RGB per pixel. It provides a MIPI DSI interface to
  the host and has a built-in LED backlight.
 
+config DRM_PANEL_JADARD_JD9365DA_H3
+   tristate "Jadard JD9365DA-H3 WXGA DSI panel"
+   depends on OF
+   depends on DRM_MIPI_DSI
+   depends on BACKLIGHT_CLASS_DEVICE
+   help
+ Say Y here if you want to enable support for Jadard JD9365DA-H3
+ WXGA MIPI DSI panel. The panel support TFT dot matrix LCD with
+ 800RGBx1280 dots at maximum.
+
 config DRM_PANEL_JDI_LT070ME05000
tristate "JDI LT070ME05000 WUXGA DSI panel"
depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index 34e717382dbb..af0b1ebdbac8 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9341) += 
panel-ilitek-ili9341.o
 obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o
 obj-$(CONFIG_DRM_PANEL_INNOLUX_EJ030NA) += panel-innolux-ej030na.o
 obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o
+obj-$(CONFIG_DRM_PANEL_JADARD_JD9365DA_H3) += panel-jadard-jd9365da-h3.o
 obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o
 obj-$(CONFIG_DRM_PANEL_JDI_R63452) += panel-jdi-fhd-r63452.o
 obj-$(CONFIG_DRM_PANEL_KHADAS_TS050) += panel-khadas-ts050.o
diff --git a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c 
b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
new file mode 100644
index ..48c1702a863b
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
@@ -0,0 +1,473 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2019 Radxa Limited
+ * Copyright (c) 2022 Edgeble AI Technologies Pvt. Ltd.
+ *
+ * Author:
+ * - Jagan Teki 
+ * - Stephen Chen 
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define JD9365DA_INIT_CMD_LEN  2
+
+struct jadard_init_cmd {
+   u8 data[JD9365DA_INIT_CMD_LEN];
+};
+
+struct jadard_panel_desc {
+   const struct drm_display_mode mode;
+   unsigned int lanes;
+   enum mipi_dsi_pixel_format format;
+   const struct jadard_init_cmd *init_cmds;
+   u32 num_init_cmds;
+};
+
+struct jadard {
+   struct drm_panel panel;
+   struct mipi_dsi_device *dsi;
+   const struct jadard_panel_desc *desc;
+
+   struct regulator *vdd;
+   struct regulator *vccio;
+   struct gpio_desc *reset;
+};
+
+static inline struct jadard *panel_to_jadard(struct drm_panel *panel)
+{
+   return container_of(panel, struct jadard, panel);
+}
+
+static int jadard_enable(struct drm_panel *panel)
+{
+   struct device *dev = panel->dev;
+   struct jadard *jadard = panel_to_jadard(panel);
+   const struct jadard_panel_desc *desc = jadard->desc;
+   struct mipi_dsi_device *dsi = jadard->dsi;
+   unsigned int i;
+   int err;
+
+   msleep(10);
+
+   for (i = 0; i < desc->num_init_cmds; i++) {
+   const struct jadard_init_cmd *cmd = &desc->init_cmds[i];
+
+   err = mipi_dsi_dcs_write_buffer(dsi, cmd->data, 
JD9365DA_INIT_CMD_LEN);
+   if (err < 0)
+   return err;
+   }
+
+   msleep(120);
+
+   err = mipi_dsi_dcs_exit_sleep_mode(dsi);
+   if (err < 0)
+   DRM_DEV_ERROR(dev, "failed to exit sleep mode ret = %d\n", err);
+
+   err =  mipi_dsi_dcs_set_display_on(dsi);
+   if (err < 0)
+   DRM_DEV_ERROR(dev, "failed to set display on ret = %d\n", err);
+
+   return 0;
+}
+
+stat

[PATCH v4 3/4] dt-bindings: display: Document Jadard JD9365DA-H3 DSI panel

Jadard JD9365DA-H3 is WXGA MIPI DSI panel and it support TFT
dot matrix LCD with 800RGBx1280 dots at maximum.

Document it.

Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Jagan Teki 
Reviewed-by: Krzysztof Kozlowski 
---
Changes for v4:
- none
Changes for v3:
- updated title
- collect Krzysztof r-b
- rebased on drm-misc-next
Changes for v2:
- none

 .../display/panel/jadard,jd9365da-h3.yaml | 70 +++
 MAINTAINERS   |  5 ++
 2 files changed, 75 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/panel/jadard,jd9365da-h3.yaml

diff --git 
a/Documentation/devicetree/bindings/display/panel/jadard,jd9365da-h3.yaml 
b/Documentation/devicetree/bindings/display/panel/jadard,jd9365da-h3.yaml
new file mode 100644
index ..c06902e4fe70
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/jadard,jd9365da-h3.yaml
@@ -0,0 +1,70 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/jadard,jd9365da-h3.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Jadard JD9365DA-HE WXGA DSI panel
+
+maintainers:
+  - Jagan Teki 
+
+allOf:
+  - $ref: panel-common.yaml#
+
+properties:
+  compatible:
+items:
+  - enum:
+  - chongzhou,cz101b4001
+  - const: jadard,jd9365da-h3
+
+  reg: true
+
+  vdd-supply:
+description: supply regulator for VDD, usually 3.3V
+
+  vccio-supply:
+description: supply regulator for VCCIO, usually 1.8V
+
+  reset-gpios: true
+
+  backlight: true
+
+  port: true
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+  - vccio-supply
+  - reset-gpios
+
+additionalProperties: false
+
+examples:
+  - |
+#include 
+#include 
+
+dsi {
+#address-cells = <1>;
+#size-cells = <0>;
+
+panel@0 {
+compatible = "chongzhou,cz101b4001", "jadard,jd9365da-h3";
+reg = <0>;
+vdd-supply = <&lcd_3v3>;
+vccio-supply = <&vcca_1v8>;
+reset-gpios = <&gpio1 RK_PC2 GPIO_ACTIVE_HIGH>;
+backlight = <&backlight>;
+
+port {
+mipi_in_panel: endpoint {
+remote-endpoint = <&mipi_out_panel>;
+};
+};
+};
+};
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 20554fd74596..8cd909cd34e0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6530,6 +6530,11 @@ S:   Orphan / Obsolete
 F: drivers/gpu/drm/i810/
 F: include/uapi/drm/i810_drm.h
 
+DRM DRIVER FOR JADARD JD9365DA-H3 MIPI-DSI LCD PANELS
+M: Jagan Teki 
+S: Maintained
+F: Documentation/devicetree/bindings/display/panel/jadard,jd9365da-h3.yaml
+
 DRM DRIVER FOR LOGICVC DISPLAY CONTROLLER
 M: Paul Kocialkowski 
 S: Supported
-- 
2.25.1

[PATCH v4 2/4] dt-bindings: vendor-prefixes: Document Jadard

Jadard Technology Inc. manufactures and distributes chips
from Shenzhen.

Add vendor prefix for it.

Signed-off-by: Jagan Teki 
Acked-by: Krzysztof Kozlowski 
---
Changes for v4, v3, v2:
- none

 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml 
b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 08264594a99f..c601f5438b02 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -645,6 +645,8 @@ patternProperties:
 description: ITian Corporation
   "^iwave,.*":
 description: iWave Systems Technologies Pvt. Ltd.
+  "^jadard,.*":
+description: Jadard Technology Inc.
   "^jdi,.*":
 description: Japan Display Inc.
   "^jedec,.*":
-- 
2.25.1

[PATCH v4 1/4] dt-bindings: vendor-prefixes: Document Chongzhou

Chongzhou is a professional is a manufacturer of LCD panels
from Shenzhen.

Add vendor prefix for it.

Acked-by: Krzysztof Kozlowski 
Signed-off-by: Jagan Teki 
---
Changes for v4, v3, v2:
- none

 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml 
b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index eae3bc8a4484..08264594a99f 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -246,6 +246,8 @@ patternProperties:
 description: ChipOne
   "^chipspark,.*":
 description: ChipSPARK
+  "^chongzhou,.*":
+description: Shenzhen Chongzhou Electronic Technology Co., Ltd
   "^chrontel,.*":
 description: Chrontel, Inc.
   "^chrp,.*":
-- 
2.25.1

[Bug 216673] Recurring amdgpu freeze on kernel 6.0.6 only

2022-11-08 Thread bugzilla-daemon

https://bugzilla.kernel.org/show_bug.cgi?id=216673

--- Comment #1 from Stanislav Modrak (stanislav.mod...@gmail.com) ---
Seems similar to
[https://gitlab.freedesktop.org/drm/amd/-/issues/2113](https://gitlab.freedesktop.org/drm/amd/-/issues/2113)

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 216673] New: Recurring amdgpu freeze on kernel 6.0.6 only

2022-11-08 Thread bugzilla-daemon

https://bugzilla.kernel.org/show_bug.cgi?id=216673

Bug ID: 216673
   Summary: Recurring amdgpu freeze on kernel 6.0.6 only
   Product: Drivers
   Version: 2.5
Kernel Version: 6.0.6-060006-generic
  Hardware: AMD
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: high
  Priority: P1
 Component: Video(DRI - non Intel)
  Assignee: drivers_video-...@kernel-bugs.osdl.org
  Reporter: stanislav.mod...@gmail.com
Regression: No

Created attachment 303150
  --> https://bugzilla.kernel.org/attachment.cgi?id=303150&action=edit
log messages

The monitor goes blank at 17:00:34 system time (the screen froze for a moment
before going blank completely, showing this timestamp) and no response from the
system with keyboard commands is visible. Forced to hard reboot Recurring
issue, only on kernel 6.0.6.

Attaching the full log file here.

Identified the log at 17:00:34 exactly. There are multiple logs at this
timestamp, however, showing just one.

```
Time: 17:00:34
Message: amdgpu :06:00.0: amdgpu: [mmhub0] no-retry page fault (src_id:0
ring:40 vmid:4 pasid:32777, for process todoist pid 26652 thread todoist:cs0
pid 26658)
Kernel Device: +pci::06:00.0
Priority: 3
```

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[PATCH] drm/panfrost: Split io-pgtable requests properly

2022-11-08 Thread Robin Murphy

Although we don't use 1GB block mappings, we still need to split
map/unmap requests at 1GB boundaries to match what io-pgtable expects.
Fix that, and add some explanation to make sense of it all.

Fixes: 3740b081795a ("drm/panfrost: Update io-pgtable API")
Reported-by: Dmitry Osipenko 
Signed-off-by: Robin Murphy 
---
The previous diff turned out to be not quite right, so I've not
included Dmitry's Tested-by given for that.
---
 drivers/gpu/drm/panfrost/panfrost_mmu.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c 
b/drivers/gpu/drm/panfrost/panfrost_mmu.c
index e246d914e7f6..4e83a1891f3e 100644
--- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
@@ -250,13 +250,22 @@ void panfrost_mmu_reset(struct panfrost_device *pfdev)
 
 static size_t get_pgsize(u64 addr, size_t size, size_t *count)
 {
+   /*
+* io-pgtable only operates on multiple pages within a single table
+* entry, so we need to split at boundaries of the table size, i.e.
+* the next block size up. The distance from address A to the next
+* boundary of block size B is logically B - A % B, but in unsigned
+* two's complement where B is a power of two we get the equivalence
+* B - A % B == (B - A) % B == (n * B - A) % B, and choose n = 0 :)
+*/
size_t blk_offset = -addr % SZ_2M;
 
if (blk_offset || size < SZ_2M) {
*count = min_not_zero(blk_offset, size) / SZ_4K;
return SZ_4K;
}
-   *count = size / SZ_2M;
+   blk_offset = -addr % SZ_1G ?: SZ_1G;
+   *count = min(blk_offset, size) / SZ_2M;
return SZ_2M;
 }
 
-- 
2.36.1.dirty

Re: [PATCH v2 03/18] dt-bindings: msm: dsi-controller-main: Add vdd* descriptions back in

2022-11-08 Thread Dmitry Baryshkov

On Tue, 8 Nov 2022 at 17:42, Bryan O'Donoghue
 wrote:
>
> On 08/11/2022 12:59, Dmitry Baryshkov wrote:
> >>
> >> Warnings about missing regulators can be resolved by updating the
> >> relevant
> >> dtsi files to point to fixed always-on regulators where appropriate.
> >
> > Ugh. Are they missing or are they optional/not used on these platforms?
>
> Some platforms either don't implement them or worse possibly do
> implement but don't model them when they should.
>
> > Can you possibly list all regulator warnings?
>
>
> Downstream we have
>
> arch/arm/boot/dts/qcom/msm8916-mdss.dtsi
>
> mdss_dsi0: qcom,mdss_dsi@1a98000 {
> vdda-supply = <&pm8916_l2>;
> vdd-supply = <&pm8916_l17>;
> vddio-supply = <&pm8916_l6>;
> };
>
> Looking at something like
>
> arch/arm/boot/dts/qcom/msm8916-mtp.dtsi which references
> arch/arm/boot/dts/qcom/dsi-panel-jdi-1080p-video.dtsi it doesn't appear
> to delete andy of the vdd*-supply references

Good example. We have vdda-supply and vddio-supply in
msm8916-pm8916.dtsi (correct).
And the mentioned vdd-supply is not applicable to this platform. It
powers up the external DSI-to-HDMI bridge. I'd assume that the same
regulator was destined to power up the panel.

So, I'd suggest defining all supplies at the top level and then
pushing all the required: parts into the per-SoC entries.

-- 
With best wishes
Dmitry

Re: [PATCH 1/2] drm/amdgpu: Move racy global PMU list into device

2022-11-08 Thread Felix Kuehling


On 2022-10-28 18:48, Brian Norris wrote:

If there are multiple amdgpu devices, this list processing can be racy.

We're really treating this like a per-device list, so make that explicit
and remove the global list.


I agree with the problem and the solution. See one comment inline.




Signed-off-by: Brian Norris 
---

  drivers/gpu/drm/amd/amdgpu/amdgpu.h |  4 
  drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 12 +---
  2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 0e6ddf05c23c..e968b7f2417c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1063,6 +1063,10 @@ struct amdgpu_device {
struct work_struct  reset_work;
  
  	booljob_hang;

+
+#if IS_ENABLED(CONFIG_PERF_EVENTS)
+   struct list_head pmu_list;
+#endif
  };
  
  static inline struct amdgpu_device *drm_to_adev(struct drm_device *ddev)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
index 71ee361d0972..24f2055a2f23 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
@@ -23,6 +23,7 @@
  
  #include 

  #include 
+#include 
  #include "amdgpu.h"
  #include "amdgpu_pmu.h"
  
@@ -72,9 +73,6 @@ static ssize_t amdgpu_pmu_event_show(struct device *dev,

amdgpu_pmu_attr->event_str, amdgpu_pmu_attr->type);
  }
  
-static LIST_HEAD(amdgpu_pmu_list);

-
-
  struct amdgpu_pmu_attr {
const char *name;
const char *config;
@@ -558,7 +556,7 @@ static int init_pmu_entry_by_type_and_add(struct 
amdgpu_pmu_entry *pmu_entry,
pr_info("Detected AMDGPU %d Perf Events.\n", total_num_events);
  
  
-	list_add_tail(&pmu_entry->entry, &amdgpu_pmu_list);

+   list_add_tail(&pmu_entry->entry, &pmu_entry->adev->pmu_list);


While you're making the pmu list per-device, I'd suggest removing adev 
from the pmu entry because it is now redundant. The device is implied by 
the list that the entry is on. Instead, add an adev parameter to 
init_pmu_entry_by_type_and_add. Or you could move the list_add_tail to 
amdgpu_pmu_init and remove "_and_add" from the function name.


Other than that, the patch looks good to me.

Regards,
  Felix


  
  	return 0;

  err_register:
@@ -579,9 +577,7 @@ void amdgpu_pmu_fini(struct amdgpu_device *adev)
  {
struct amdgpu_pmu_entry *pe, *temp;
  
-	list_for_each_entry_safe(pe, temp, &amdgpu_pmu_list, entry) {

-   if (pe->adev != adev)
-   continue;
+   list_for_each_entry_safe(pe, temp, &adev->pmu_list, entry) {
list_del(&pe->entry);
perf_pmu_unregister(&pe->pmu);
kfree(pe->pmu.attr_groups);
@@ -623,6 +619,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
int ret = 0;
struct amdgpu_pmu_entry *pmu_entry, *pmu_entry_df;
  
+	INIT_LIST_HEAD(&adev->pmu_list);

+
switch (adev->asic_type) {
case CHIP_VEGA20:
pmu_entry_df = create_pmu_entry(adev, AMDGPU_PMU_PERF_TYPE_DF,

RE: [EXT] Re: [PATCH v3 00/10] Initial support for Cadence MHDP(HDMI/DP) for i.MX8MQ

2022-11-08 Thread Jani Nikula

On Tue, 08 Nov 2022, Sandor Yu  wrote:
> Run get_maintainer.pl for patch 1, 
> Your email address is list as follow,
> Jani Nikula  (commit_signer:2/8=25%)
>
> And I add the email address that comment as "commit_signer" into Cc list.

Please don't do that.

get_maintainer.pl gives you no less than *eight* people as designated
maintainers or reviewers.

I've acked two commits with one-line collateral changes to the files in
question.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [PATCH v3 4/4] drm: panel: Add Jadard JD9365DA-H3 DSI panel

On Tue, 8 Nov 2022 at 20:33, Linus Walleij  wrote:
>
> On Tue, Nov 8, 2022 at 3:53 PM Jagan Teki  wrote:
> > On Tue, 8 Nov 2022 at 20:18, Linus Walleij  wrote:
> > > On Tue, Nov 8, 2022 at 3:12 PM Jagan Teki  wrote:
> > > > On Tue, 8 Nov 2022 at 19:31, Linus Walleij  
> > > > wrote:
> > > > >
> > > > > On Thu, Nov 3, 2022 at 3:12 PM Jagan Teki  wrote:
> > > > >
> > > > > > Jadard JD9365DA-H3 is WXGA MIPI DSI panel and it support TFT
> > > > > > dot matrix LCD with 800RGBx1280 dots at maximum.
> > > > > >
> > > > > > Add support for it.
> > > > > >
> > > > > > Cc: dri-devel@lists.freedesktop.org
> > > > > > Signed-off-by: Jagan Teki 
> > > > > > ---
> > > > > > Changes for v3:
> > > > > > - updatd to WXGA
> > > > > > - use JD9365DA_CMD_DCS and JD9365DA_CMD_DELAY
> > > > >
> > > > > My comments on v2 have not been addressed, for example I asked to
> > > > > remove the delay from sequences and just use an explicit delay and
> > > > > to then use the existing sequence sending macro.
> > > >
> > > > True, I responded on the same day [1], since I didn't get the reply I
> > > > have posted by assuming my comment is valid. Would you please check
> > > > and respond?
> > > >
> > > > [1] 
> > > > https://lore.kernel.org/all/CA+VMnFz0w-6O=wt3iujo1bhqgpz2xbpx6jddz6vg_jw9nht...@mail.gmail.com/
> > >
> > > OK I see, sorry for not reading close.
> > >
> > > The driver just supports one single variant.
> > >
> > > What you are doing is preparing the ground for more variants
> > > that may or may not exist. This creates the antipattern "big upfront 
> > > design"
> > > i.e. abstractions added for things that do not yet exist.
> > >
> > > I think it is better to strip it down to just open coding the delay after
> > > the init sequence. When the next variant appears, if ever, they can
> > > add abstraction. Maybe they need the same delay in the same
> > > place? Who knows...
> >
> > I understand your point, but delays are strictly mentioned by the
> > panel vendor init sequence, cz101b4001 do you think adding in the
> > generic or common code is still valid since we have code added for
> > cz101b4001 specifically via driver data?
>
> I would instead of encoding a sequence into the driver data
> encode a per-variant callback that sends the sequence (which
> can then be just static array) and then ends with an explicit delay.

Okay. I will wrap up that delay to make it explicit and update the dcs
packet send sequence.

Thanks,
Jagan.

Re: [PATCH] drm/amd/display: Amend descriptions within enum pipe_split_policy

On Mon, Nov 7, 2022 at 9:37 AM Carlos Bilbao  wrote:
>
> Correct descriptions of two last fields of enum pipe_split_policy, updating
> comments with proper field names.
>
> Signed-off-by: Carlos Bilbao 

Acked-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/display/dc/dc.h | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dc.h 
> b/drivers/gpu/drm/amd/display/dc/dc.h
> index bfc5474c0f4c..277631a899d8 100644
> --- a/drivers/gpu/drm/amd/display/dc/dc.h
> +++ b/drivers/gpu/drm/amd/display/dc/dc.h
> @@ -457,15 +457,16 @@ enum pipe_split_policy {
> MPC_SPLIT_DYNAMIC = 0,
>
> /**
> -* @MPC_SPLIT_DYNAMIC: Avoid pipe split, which means that DC will not
> +* @MPC_SPLIT_AVOID: Avoid pipe split, which means that DC will not
>  * try any sort of split optimization.
>  */
> MPC_SPLIT_AVOID = 1,
>
> /**
> -* @MPC_SPLIT_DYNAMIC: With this option, DC will only try to optimize
> -* the pipe utilization when using a single display; if the user
> -* connects to a second display, DC will avoid pipe split.
> +* @MPC_SPLIT_AVOID_MULT_DISP: With this option, DC will only try
> +* to optimize the pipe utilization when using a single display;
> +* if the user connects to a second display, DC will avoid pipe
> +* split.
>  */
> MPC_SPLIT_AVOID_MULT_DISP = 2,
>  };
> --
> 2.34.1
>

Re: [PATCH RESEND] drm: fix crash in drm_minor_alloc_release

2022-11-08 Thread Stanislaw Gruszka

On Mon, Nov 07, 2022 at 05:56:36PM +0200, Ville Syrjälä wrote:
> On Mon, Nov 07, 2022 at 04:40:41PM +0100, Stanislaw Gruszka wrote:
> > On Mon, Nov 07, 2022 at 05:10:48PM +0200, Ville Syrjälä wrote:
> > > On Mon, Nov 07, 2022 at 03:45:00PM +0100, Stanislaw Gruszka wrote:
> > > > index 8214a0b1ab7f..e3a1243dd2ae 100644
> > > > --- a/drivers/gpu/drm/drm_drv.c
> > > > +++ b/drivers/gpu/drm/drm_drv.c
> > > > @@ -102,7 +102,8 @@ static void drm_minor_alloc_release(struct 
> > > > drm_device *dev, void *data)
> > > >  
> > > > WARN_ON(dev != minor->dev);
> > > >  
> > > > -   put_device(minor->kdev);
> > > > +   if (!IS_ERR(minor->kdev))
> > > > +   put_device(minor->kdev);
> > > 
> > > Assigning error pointers into things is a terrible idea.
> > > IMO the correct fix would be to not return some
> > > half-constructed garbage from drm_minor_alloc().
> > > So basically should at least partically revert
> > > commit f96306f9892b ("drm: manage drm_minor cleanup with drmm_")
> > 
> > I would prefer to not change any ordering or remove drmm_* stuff, since
> > as pointed to above commit message, things are tricky there.
> 
> Looks to me that it's only tricky because of drmm. Without that it was
> totally clear what was happening. I think if the managed stuff is making
> stuff more tricky then it has failed its purpose.

I'm not huge fan of managed resources everywhere either, but I think
we should do rather small fixes for bugs to avoid regressions.

> > I think assigning NULL to minor->kdev should be fine:
> > 
> > if (IS_ERR(minor->kdev)) {
> > r = PTR_ERR(minor->kdev);
> > minor->kdev = NULL;
> > return r;
> > }

Seems having minor->kdev NULL was ordinal Daniel idea in commit
f96306f9892b, but was not done properly when finally patch get's in.

Regards
Stanislaw

Re: [RFC PATCH v3 2/3] accel: add dedicated minor for accelerator devices

2022-11-08 Thread Oded Gabbay

On Tue, Nov 8, 2022 at 3:13 PM Tvrtko Ursulin
 wrote:
>
>
> On 06/11/2022 21:02, Oded Gabbay wrote:
> > The accelerator devices are exposed to user-space using a dedicated
> > major. In addition, they are represented in /dev with new, dedicated
> > device char names: /dev/accel/accel*. This is done to make sure any
> > user-space software that tries to open a graphic card won't open
> > the accelerator device by mistake.
> >
> > The above implies that the minor numbering should be separated from
> > the rest of the DRM devices. However, to avoid code duplication, we
> > want the drm_minor structure to be able to represent the accelerator
> > device.
> >
> > To achieve this, we add a new drm_minor* to drm_device that represents
> > the accelerator device. This pointer is initialized for drivers that
> > declare they handle compute accelerator, using a new driver feature
> > flag called DRIVER_COMPUTE_ACCEL. It is important to note that this
> > driver feature is mutually exclusive with DRIVER_RENDER. Devices that
> > want to expose both graphics and compute device char files should be
> > handled by two drivers that are connected using the auxiliary bus
> > framework.
> >
> > In addition, we define a different IDR to handle the accelerators
> > minors. This is done to make the minor's index be identical to the
> > device index in /dev/. Any access to the IDR is done solely
> > by functions in accel_drv.c, as the IDR is define as static. The
> > DRM core functions call those functions in case they detect the minor's
> > type is DRM_MINOR_ACCEL.
> >
> > We define a separate accel_open function (from drm_open) that the
> > accel drivers should set as their open callback function. Both these
> > functions eventually call the same drm_open_helper(), which had to be
> > changed to be non-static so it can be called from accel_drv.c.
> > accel_open() only partially duplicates drm_open as I removed some code
> > from it that handles legacy devices.
> >
> > To help new drivers, I defined DEFINE_DRM_ACCEL_FOPS macro to easily
> > set the required function operations pointers structure.
> >
> > Signed-off-by: Oded Gabbay 
> > ---
> > Changes in v3:
> >   - Remove useless DRM_DEBUG("\n") at accel_stub_open()
> >   - Add function of accel_debugfs_init() as accel_debugfs_root is static
> > member in drm_accel.c
> >   - Add DRM_ACCEL_FOPS and DEFINE_DRM_ACCEL_FOPS macros
> >   - Replace minor handling from xarray back to idr, as xarray doesn't handle
> > well exchanging content of a NULL entry to non-NULL. This should be 
> > handled
> > in a different patch that will either fix xarray code or change DRM 
> > minor
> > init flow.
> >   - Make accel_minor_replace() to return void.
> >
> >   drivers/accel/drm_accel.c  | 242 -
> >   drivers/gpu/drm/drm_file.c |   2 +-
> >   include/drm/drm_accel.h|  68 ++-
> >   include/drm/drm_device.h   |   3 +
> >   include/drm/drm_drv.h  |   8 ++
> >   include/drm/drm_file.h |  21 +++-
> >   6 files changed, 340 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/accel/drm_accel.c b/drivers/accel/drm_accel.c
> > index 943d960ddefc..05167c929866 100644
> > --- a/drivers/accel/drm_accel.c
> > +++ b/drivers/accel/drm_accel.c
> > @@ -8,14 +8,25 @@
> >
> >   #include 
> >   #include 
> > +#include 
> >
> >   #include 
> > +#include 
> > +#include 
> > +#include 
> >   #include 
> >   #include 
> >
> > +static DEFINE_SPINLOCK(accel_minor_lock);
> > +static struct idr accel_minors_idr;
> > +
> >   static struct dentry *accel_debugfs_root;
> >   static struct class *accel_class;
> >
> > +static struct device_type accel_sysfs_device_minor = {
> > + .name = "accel_minor"
> > +};
> > +
> >   static char *accel_devnode(struct device *dev, umode_t *mode)
> >   {
> >   return kasprintf(GFP_KERNEL, "accel/%s", dev_name(dev));
> > @@ -40,9 +51,235 @@ static void accel_sysfs_destroy(void)
> >   accel_class = NULL;
> >   }
> >
> > +static int accel_name_info(struct seq_file *m, void *data)
> > +{
> > + struct drm_info_node *node = (struct drm_info_node *) m->private;
> > + struct drm_minor *minor = node->minor;
> > + struct drm_device *dev = minor->dev;
> > + struct drm_master *master;
> > +
> > + mutex_lock(&dev->master_mutex);
> > + master = dev->master;
> > + seq_printf(m, "%s", dev->driver->name);
> > + if (dev->dev)
> > + seq_printf(m, " dev=%s", dev_name(dev->dev));
> > + if (master && master->unique)
> > + seq_printf(m, " master=%s", master->unique);
>
> Does the all drm_master business apply with accel?
No, it was left by mistake, I'll remove it.

>
> > + if (dev->unique)
> > + seq_printf(m, " unique=%s", dev->unique);
> > + seq_puts(m, "\n");
> > + mutex_unlock(&dev->master_mutex);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct drm_info_list accel_debugfs_list[] = {
> > + {"name", accel_name_info, 0}
> >

Re: [PATCH 1/2] drm/amdgpu: Move racy global PMU list into device

On Fri, Oct 28, 2022 at 6:48 PM Brian Norris  wrote:
>
> If there are multiple amdgpu devices, this list processing can be racy.
>
> We're really treating this like a per-device list, so make that explicit
> and remove the global list.
>
> Signed-off-by: Brian Norris 

@Kuehling, Felix @Kim, Jonathan can you take a look at this patch?

Thanks,

Alex


> ---
>
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h |  4 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 12 +---
>  2 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 0e6ddf05c23c..e968b7f2417c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1063,6 +1063,10 @@ struct amdgpu_device {
> struct work_struct  reset_work;
>
> booljob_hang;
> +
> +#if IS_ENABLED(CONFIG_PERF_EVENTS)
> +   struct list_head pmu_list;
> +#endif
>  };
>
>  static inline struct amdgpu_device *drm_to_adev(struct drm_device *ddev)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> index 71ee361d0972..24f2055a2f23 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> @@ -23,6 +23,7 @@
>
>  #include 
>  #include 
> +#include 
>  #include "amdgpu.h"
>  #include "amdgpu_pmu.h"
>
> @@ -72,9 +73,6 @@ static ssize_t amdgpu_pmu_event_show(struct device *dev,
> amdgpu_pmu_attr->event_str, amdgpu_pmu_attr->type);
>  }
>
> -static LIST_HEAD(amdgpu_pmu_list);
> -
> -
>  struct amdgpu_pmu_attr {
> const char *name;
> const char *config;
> @@ -558,7 +556,7 @@ static int init_pmu_entry_by_type_and_add(struct 
> amdgpu_pmu_entry *pmu_entry,
> pr_info("Detected AMDGPU %d Perf Events.\n", 
> total_num_events);
>
>
> -   list_add_tail(&pmu_entry->entry, &amdgpu_pmu_list);
> +   list_add_tail(&pmu_entry->entry, &pmu_entry->adev->pmu_list);
>
> return 0;
>  err_register:
> @@ -579,9 +577,7 @@ void amdgpu_pmu_fini(struct amdgpu_device *adev)
>  {
> struct amdgpu_pmu_entry *pe, *temp;
>
> -   list_for_each_entry_safe(pe, temp, &amdgpu_pmu_list, entry) {
> -   if (pe->adev != adev)
> -   continue;
> +   list_for_each_entry_safe(pe, temp, &adev->pmu_list, entry) {
> list_del(&pe->entry);
> perf_pmu_unregister(&pe->pmu);
> kfree(pe->pmu.attr_groups);
> @@ -623,6 +619,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
> int ret = 0;
> struct amdgpu_pmu_entry *pmu_entry, *pmu_entry_df;
>
> +   INIT_LIST_HEAD(&adev->pmu_list);
> +
> switch (adev->asic_type) {
> case CHIP_VEGA20:
> pmu_entry_df = create_pmu_entry(adev, AMDGPU_PMU_PERF_TYPE_DF,
> --
> 2.38.1.273.g43a17bfeac-goog
>

Re: [PATCH] drm/qxl: Fix missing free_irq

On Tue, Nov 08, 2022 at 11:16:01PM +0800, Wei Li wrote:
> When doing "cat /proc/interrupts" after qxl.ko is unloaded, an oops occurs:
> 
> BUG: unable to handle page fault for address: c0274769
> PGD 2a0d067 P4D 2a0d067 PUD 2a0f067 PMD 103f39067 PTE 0
> Oops:  [#1] PREEMPT SMP PTI
> CPU: 6 PID: 246 Comm: cat Not tainted 6.1.0-rc2 #24
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 
> 04/01/2014
> RIP: 0010:string_nocheck+0x34/0x50
> Code: 66 85 c0 74 3c 83 e8 01 4c 8d 5c 07 01 31 c0 eb 19 49 39 fa 76 03 44 88 
> 07 48 83 c7
> RSP: 0018:c9893bb8 EFLAGS: 00010046
> RAX:  RBX: c9893c50 RCX: 0a00ff04
> RDX: c0274769 RSI: 888102812000 RDI: 88810281133e
> RBP: 888102812000 R08: 823fa5e6 R09: 0007
> R10: 888102812000 R11: 88820281133d R12: c0274769
> R13: 0a00ff04 R14: 0cc4 R15: 823276b4
> FS:  0214f8c0() GS:88842fd8() knlGS:
> CS:  0010 DS:  ES:  CR0: 80050033
> CR2: c0274769 CR3: 0001025c4005 CR4: 00770ee0
> DR0:  DR1:  DR2: 
> DR3:  DR6: fffe0ff0 DR7: 0400
> PKRU: 5554
> Call Trace:
>  
>  string+0x46/0x60
>  vsnprintf+0x27a/0x4f0
>  seq_vprintf+0x34/0x50
>  seq_printf+0x53/0x70
>  ? seq_read_iter+0x365/0x450
>  show_interrupts+0x259/0x330
>  seq_read_iter+0x2a3/0x450
>  proc_reg_read_iter+0x47/0x70
>  generic_file_splice_read+0x94/0x160
>  splice_direct_to_actor+0xb0/0x230
>  ? do_splice_direct+0xd0/0xd0
>  do_splice_direct+0x8b/0xd0
>  do_sendfile+0x345/0x4f0
>  __x64_sys_sendfile64+0xa1/0xc0
>  do_syscall_64+0x38/0x90
>  entry_SYSCALL_64_after_hwframe+0x63/0xcd
> RIP: 0033:0x4bb0ce
> Code: c3 0f 1f 00 4c 89 d2 4c 89 c6 e9 bd fd ff ff 0f 1f 44 00 00 31 c0 c3 0f 
> 1f 44 00 00
> RSP: 002b:7ffd99dc3fb8 EFLAGS: 0246 ORIG_RAX: 0028
> RAX: ffda RBX: 0100 RCX: 004bb0ce
> RDX:  RSI: 0003 RDI: 0001
> RBP: 0001 R08: 0068f240 R09: 0100
> R10: 0100 R11: 0246 R12: 0003
> R13: 0001 R14:  R15: 
>  
> 
> It seems that qxl doesn't free the interrupt it requests during unload,
> fix this by adding the missing free_irq().
> 
> Fixes: f64122c1f6ad ("drm: add new QXL driver. (v1.4)")
> Signed-off-by: Wei Li 

Could we go right ahead and switch over to devm_request_irq? Or does that
not quite do the right thing here?
-Daniel

> ---
>  drivers/gpu/drm/qxl/qxl_kms.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
> index dc3828db1991..d591084824de 100644
> --- a/drivers/gpu/drm/qxl/qxl_kms.c
> +++ b/drivers/gpu/drm/qxl/qxl_kms.c
> @@ -283,6 +283,8 @@ int qxl_device_init(struct qxl_device *qdev,
>  void qxl_device_fini(struct qxl_device *qdev)
>  {
>   int cur_idx;
> + struct drm_device *ddev = &qdev->ddev;
> + struct pci_dev *pdev = to_pci_dev(ddev->dev);
>  
>   /* check if qxl_device_init() was successful (gc_work is initialized 
> last) */
>   if (!qdev->gc_work.func)
> @@ -305,6 +307,7 @@ void qxl_device_fini(struct qxl_device *qdev)
>   wait_event_timeout(qdev->release_event,
>  atomic_read(&qdev->release_count) == 0,
>  HZ);
> + free_irq(pdev->irq, ddev);
>   flush_work(&qdev->gc_work);
>   qxl_surf_evict(qdev);
>   qxl_vram_evict(qdev);
> -- 
> 2.25.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 2/2] drm/v3d: add missing mutex_destroy

On Mon, Nov 07, 2022 at 07:46:56PM -0300, Maíra Canal wrote:
> v3d_perfmon_open_file() instantiates a mutex for a particular file
> instance, but it never destroys it by calling mutex_destroy() in
> v3d_perfmon_close_file().
> 
> Similarly, v3d_perfmon_create_ioctl() instantiates a mutex for a
> particular perfmon, but it never destroys it by calling mutex_destroy()
> in v3d_perfmon_destroy_ioctl().
> 
> So, add the missing mutex_destroy on both cases.
> 
> Signed-off-by: Maíra Canal 
> ---
>  drivers/gpu/drm/v3d/v3d_perfmon.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/v3d/v3d_perfmon.c 
> b/drivers/gpu/drm/v3d/v3d_perfmon.c
> index 48972c49..292c73544255 100644
> --- a/drivers/gpu/drm/v3d/v3d_perfmon.c
> +++ b/drivers/gpu/drm/v3d/v3d_perfmon.c
> @@ -113,6 +113,7 @@ void v3d_perfmon_close_file(struct v3d_file_priv 
> *v3d_priv)
>   idr_for_each(&v3d_priv->perfmon.idr, v3d_perfmon_idr_del, NULL);
>   idr_destroy(&v3d_priv->perfmon.idr);
>   mutex_unlock(&v3d_priv->perfmon.lock);
> + mutex_destroy(&v3d_priv->perfmon.lock);
>  }
>  
>  int v3d_perfmon_create_ioctl(struct drm_device *dev, void *data,
> @@ -177,6 +178,7 @@ int v3d_perfmon_destroy_ioctl(struct drm_device *dev, 
> void *data,
>   if (!perfmon)
>   return -EINVAL;
>  
> + mutex_destroy(&perfmon->lock);

This one looks wrong, I think this should be put into v3d_perfmon_put.
There is also an error case in v3d_perfmon_create_ioctl() that I think you
missed, right above the kfree(perfmon).

With these two fixes this is Reviewed-by: Daniel Vetter 


>   v3d_perfmon_put(perfmon);
>  
>   return 0;
> -- 
> 2.38.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH] [next] drm/amdgpu: Replace 1-element array with flexible-array member

Applied.  Thanks!

Alex

On Mon, Nov 7, 2022 at 4:33 AM Paulo Miguel Almeida
 wrote:
>
> One-element arrays are deprecated, and we are replacing them with
> flexible array members instead. So, replace one-element array with
> flexible-array member in structs _ATOM_GPIO_PIN_ASSIGNMENT,
> _ATOM_DISPLAY_OBJECT_PATH, _ATOM_DISPLAY_OBJECT_PATH_TABLE,
> _ATOM_OBJECT_TABLE and refactor the rest of the code accordingly.
>
> Important to mention is that doing a build before/after this patch results
> in no functional binary output differences.
>
> This helps with the ongoing efforts to tighten the FORTIFY_SOURCE
> routines on memcpy() and help us make progress towards globally
> enabling -fstrict-flex-arrays=3 [1].
>
> Link: https://github.com/KSPP/linux/issues/79
> Link: https://github.com/KSPP/linux/issues/238
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1]
>
> Signed-off-by: Paulo Miguel Almeida 
> ---
>
> Binary difference findings:
>
> Some changes took more than a single line which changed the line
> number parameter passed to the drm_dbg function (which leverages
> kernel's dynamic debugging). Functionally-wise, nothing changed
> after doing a before/after patch build.
>
> Additional one-element arrays to be changed:
>
> There are more instances of one-element arrays to be changed but
> I will keep patches small so they are easy to review. [and I can
> only dedicate a few hours per day on this :-) ]
>
> ---
>  .../gpu/drm/amd/display/dc/bios/bios_parser.c | 23 ---
>  drivers/gpu/drm/amd/include/atombios.h|  8 +++
>  2 files changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c 
> b/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c
> index 9b8ea6e9a2b9..39dd8b2dc254 100644
> --- a/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c
> +++ b/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c
> @@ -138,7 +138,9 @@ static uint8_t get_number_of_objects(struct bios_parser 
> *bp, uint32_t offset)
>
> uint32_t object_table_offset = bp->object_info_tbl_offset + offset;
>
> -   table = GET_IMAGE(ATOM_OBJECT_TABLE, object_table_offset);
> +   table = ((ATOM_OBJECT_TABLE *) bios_get_image(&bp->base,
> +   object_table_offset,
> +   struct_size(table, asObjects, 1)));
>
> if (!table)
> return 0;
> @@ -166,8 +168,9 @@ static struct graphics_object_id 
> bios_parser_get_connector_id(
> uint32_t connector_table_offset = bp->object_info_tbl_offset
> + 
> le16_to_cpu(bp->object_info_tbl.v1_1->usConnectorObjectTableOffset);
>
> -   ATOM_OBJECT_TABLE *tbl =
> -   GET_IMAGE(ATOM_OBJECT_TABLE, connector_table_offset);
> +   ATOM_OBJECT_TABLE *tbl = ((ATOM_OBJECT_TABLE *) 
> bios_get_image(&bp->base,
> +   connector_table_offset,
> +   struct_size(tbl, asObjects, 1)));
>
> if (!tbl) {
> dm_error("Can't get connector table from atom bios.\n");
> @@ -1789,11 +1792,13 @@ static enum bp_result bios_parser_get_gpio_pin_info(
> if (!DATA_TABLES(GPIO_Pin_LUT))
> return BP_RESULT_BADBIOSTABLE;
>
> -   header = GET_IMAGE(ATOM_GPIO_PIN_LUT, DATA_TABLES(GPIO_Pin_LUT));
> +   header = ((ATOM_GPIO_PIN_LUT *) bios_get_image(&bp->base,
> +   DATA_TABLES(GPIO_Pin_LUT),
> +   struct_size(header, asGPIO_Pin, 1)));
> if (!header)
> return BP_RESULT_BADBIOSTABLE;
>
> -   if (sizeof(ATOM_COMMON_TABLE_HEADER) + sizeof(ATOM_GPIO_PIN_LUT)
> +   if (sizeof(ATOM_COMMON_TABLE_HEADER) + struct_size(header, 
> asGPIO_Pin, 1)
> > le16_to_cpu(header->sHeader.usStructureSize))
> return BP_RESULT_BADBIOSTABLE;
>
> @@ -1978,7 +1983,8 @@ static ATOM_OBJECT *get_bios_object(struct bios_parser 
> *bp,
>
> offset += bp->object_info_tbl_offset;
>
> -   tbl = GET_IMAGE(ATOM_OBJECT_TABLE, offset);
> +   tbl = ((ATOM_OBJECT_TABLE *) bios_get_image(&bp->base, offset,
> +   struct_size(tbl, asObjects, 1)));
> if (!tbl)
> return NULL;
>
> @@ -2709,8 +2715,9 @@ static enum bp_result get_bracket_layout_record(
>
> genericTableOffset = bp->object_info_tbl_offset +
> bp->object_info_tbl.v1_3->usMiscObjectTableOffset;
> -   object_table = (ATOM_OBJECT_TABLE *)
> -   GET_IMAGE(ATOM_OBJECT_TABLE, genericTableOffset);
> +   object_table = ((ATOM_OBJECT_TABLE *) bios_get_image(&bp->base,
> +   genericTableOffset,
> +   struct_size(object_table, asObjects, 1)));
> if (!object_table)
> return BP_RESULT_FAILURE;
>
> diff --git a/drivers/gpu/drm/amd/include/atombios.h 
> b/dri

Re: [PATCH 1/2] drm/v3d: switch to drmm_mutex_init

On Mon, Nov 07, 2022 at 07:46:55PM -0300, Maíra Canal wrote:
> mutex_init() is supposed to be balanced by a call to mutex_destroy(), but
> this is not currently happening on the v3d driver.
> 
> Considering the introduction of a DRM-managed mutex_init variant, switch
> to drmm_mutex_init.
> 
> Signed-off-by: Maíra Canal 

Reviewed-by: Daniel Vetter 

> ---
>  drivers/gpu/drm/v3d/v3d_gem.c | 17 +
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
> index b8980440d137..96af1cb5202a 100644
> --- a/drivers/gpu/drm/v3d/v3d_gem.c
> +++ b/drivers/gpu/drm/v3d/v3d_gem.c
> @@ -10,6 +10,7 @@
>  #include 
>  #include 
>  
> +#include 
>  #include 
>  #include 
>  
> @@ -1075,10 +1076,18 @@ v3d_gem_init(struct drm_device *dev)
>  
>   spin_lock_init(&v3d->mm_lock);
>   spin_lock_init(&v3d->job_lock);
> - mutex_init(&v3d->bo_lock);
> - mutex_init(&v3d->reset_lock);
> - mutex_init(&v3d->sched_lock);
> - mutex_init(&v3d->cache_clean_lock);
> + ret = drmm_mutex_init(dev, &v3d->bo_lock);
> + if (ret)
> + return ret;
> + ret = drmm_mutex_init(dev, &v3d->reset_lock);
> + if (ret)
> + return ret;
> + ret = drmm_mutex_init(dev, &v3d->sched_lock);
> + if (ret)
> + return ret;
> + ret = drmm_mutex_init(dev, &v3d->cache_clean_lock);
> + if (ret)
> + return ret;
>  
>   /* Note: We don't allocate address 0.  Various bits of HW
>* treat 0 as special, such as the occlusion query counters
> -- 
> 2.38.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v2 0/7] drm: Review of mode copies

On Mon, Nov 07, 2022 at 09:25:38PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Repost of the stragglers from
> https://patchwork.freedesktop.org/series/100393/
> 
> Note that I didn't rerun the cocci stuff, nor have I had
> time to come up with something to inluce the cocci scripts
> in the tree. So it's possible this is missing some new
> things that have snuck in the meantime.
> 
> Ville Syrjälä (7):
>   drm/amdgpu: Use drm_mode_init() for on-stack modes
>   drm/hisilicon: Use drm_mode_init() for on-stack modes
>   drm/msm: Use drm_mode_init() for on-stack modes
>   drm/msm: Use drm_mode_copy()
>   drm/mtk: Use drm_mode_init() for on-stack modes
>   drm/rockchip: Use drm_mode_copy()
>   drm/sti: Use drm_mode_copy()

On the series:

Reviewed-by: Daniel Vetter 

Imo ping Alex/Rob and stuff everything left into drm-misc-next.
-Daniel

> 
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c| 3 ++-
>  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c | 2 +-
>  drivers/gpu/drm/mediatek/mtk_hdmi.c  | 2 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 7 +--
>  drivers/gpu/drm/msm/dp/dp_display.c  | 2 +-
>  drivers/gpu/drm/rockchip/cdn-dp-core.c   | 2 +-
>  drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +-
>  drivers/gpu/drm/rockchip/rk3066_hdmi.c   | 2 +-
>  drivers/gpu/drm/sti/sti_dvo.c| 2 +-
>  drivers/gpu/drm/sti/sti_hda.c| 2 +-
>  drivers/gpu/drm/sti/sti_hdmi.c   | 2 +-
>  11 files changed, 16 insertions(+), 12 deletions(-)
> 
> -- 
> 2.37.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v6 20/20] drm/i915/vm_bind: Async vm_unbind support

2022-11-08 Thread Niranjana Vishwanathapura


On Mon, Nov 07, 2022 at 05:39:34PM -0800, Zanoni, Paulo R wrote:

On Mon, 2022-11-07 at 00:52 -0800, Niranjana Vishwanathapura wrote:

Asynchronously unbind the vma upon vm_unbind call.
Fall back to synchronous unbind if backend doesn't support
async unbind or if async unbind fails.

No need for vm_unbind out fence support as i915 will internally
handle all sequencing and user need not try to sequence any
operation with the unbind completion.


Can you please provide some more details on how this works from the
user space point of view? I want to be able to know with 100% certainty
if an unbind has already happened, so I can reuse that vma or whatever
else I may decide to do. I see the interface does not provide any sort
of drm_syncobjs for me to wait on the async unbind. So, when does the
unbind really happen? When can I be sure it's past so I can do stuff
with it? Why would you provide an async ioctl and provide no means for
user space to wait on it?



Paulo,
The async vm_unbind here is not transparent to user space. From user space
point of view, it is like synchronous and they can reuse the assigned virtual
address immediately after vm_unbind ioctl returns. The i915 driver will
ensure that the unbind completes before there is a rebind at that virtual
address. So, unless there is error from user programming where GPU tries to
access the buffer even after user doing the vm_unbind, it should be fine.

Regards,
Niranjana


Thanks,
Paulo



Signed-off-by: Niranjana Vishwanathapura 
---
 drivers/gpu/drm/i915/i915_vma.c | 51 ++---
 drivers/gpu/drm/i915/i915_vma.h |  1 +
 2 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 08218e3a2f12..03c966fad87b 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -42,6 +42,8 @@
 #include "i915_vma.h"
 #include "i915_vma_resource.h"
 



+static struct dma_fence *__i915_vma_unbind_async(struct i915_vma *vma);
+
 static inline void assert_vma_held_evict(const struct i915_vma *vma)
 {
    /*
@@ -1711,7 +1713,7 @@ void i915_vma_reopen(struct i915_vma *vma)
    spin_unlock_irq(>->closed_lock);
 }
 



-static void force_unbind(struct i915_vma *vma)
+static void force_unbind(struct i915_vma *vma, bool async)
 {
    if (!drm_mm_node_allocated(&vma->node))
    return;
@@ -1725,7 +1727,21 @@ static void force_unbind(struct i915_vma *vma)
    i915_vma_set_purged(vma);
 



    atomic_and(~I915_VMA_PIN_MASK, &vma->flags);
-   WARN_ON(__i915_vma_unbind(vma));
+   if (async) {
+   struct dma_fence *fence;
+
+   fence = __i915_vma_unbind_async(vma);
+   if (IS_ERR_OR_NULL(fence)) {
+   async = false;
+   } else {
+   dma_resv_add_fence(vma->obj->base.resv, fence,
+  DMA_RESV_USAGE_READ);
+   dma_fence_put(fence);
+   }
+   }
+
+   if (!async)
+   WARN_ON(__i915_vma_unbind(vma));
    GEM_BUG_ON(drm_mm_node_allocated(&vma->node));
 }
 



@@ -1785,7 +1801,7 @@ void i915_vma_destroy_locked(struct i915_vma *vma)
 {
    lockdep_assert_held(&vma->vm->mutex);
 



-   force_unbind(vma);
+   force_unbind(vma, false);
    list_del_init(&vma->vm_link);
    release_references(vma, vma->vm->gt, false);
 }
@@ -1796,7 +1812,34 @@ void i915_vma_destroy(struct i915_vma *vma)
    bool vm_ddestroy;
 



    mutex_lock(&vma->vm->mutex);
-   force_unbind(vma);
+   force_unbind(vma, false);
+   list_del_init(&vma->vm_link);
+   vm_ddestroy = vma->vm_ddestroy;
+   vma->vm_ddestroy = false;
+
+   /* vma->vm may be freed when releasing vma->vm->mutex. */
+   gt = vma->vm->gt;
+   mutex_unlock(&vma->vm->mutex);
+   release_references(vma, gt, vm_ddestroy);
+}
+
+void i915_vma_destroy_async(struct i915_vma *vma)
+{
+   bool vm_ddestroy, async = vma->obj->mm.rsgt;
+   struct intel_gt *gt;
+
+   if (dma_resv_reserve_fences(vma->obj->base.resv, 1))
+   async = false;
+
+   mutex_lock(&vma->vm->mutex);
+   /*
+* Ensure any asynchronous binding is complete while using
+* async unbind as we will be releasing the vma here.
+*/
+   if (async && i915_active_wait(&vma->active))
+   async = false;
+
+   force_unbind(vma, async);
    list_del_init(&vma->vm_link);
    vm_ddestroy = vma->vm_ddestroy;
    vma->vm_ddestroy = false;
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 737ef310d046..25f15965dab8 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -272,6 +272,7 @@ void i915_vma_reopen(struct i915_vma *vma);
 



 void i915_vma_destroy_locked(struct i915_vma *vma);
 void i915_vma_destroy(struct i915_vma *vma);
+void i915_vma_des

Re: [PATCH] drm/edid/firmware: stop using throwaway platform device

2022-11-08 Thread Matthieu CHARETTE

I didn't test the patch yet. I will do. But even without testing I can
tell you that it will work (It will not crash).
Currently when the crash occurs, all screens remain black after resume.
I'm not able to login with ssh neither. And logs end before the
suspend. So the crash seems to be some kind of kernel panic.

Matthieu

On Tue, Nov 8 2022 at 01:27:33 PM +0200, Jani Nikula
wrote:
On Sun, 06 Nov 2022, Matthieu CHARETTE
wrote:

Hi,

Can you tell me what are we waiting for? Maybe I can help.

Have you tried the patch? Is it an improvement over the status quo?

The "crash" is still ambiguous to me. Do you observe it with the
patch?

Do you have logs? Etc.

BR,
Jani.

Thanks.

Matthieu

On Wed, Oct 12 2022 at 07:16:29 PM +0200, Matthieu CHARETTE
wrote:

By crash, I mean that an error is returned here:

https://kernel.googlesource.com/pub/scm/linux/kernel/git/torvalds/linux.git/+/refs/heads/master/drivers/gpu/drm/drm_edid_load.c#195
I don't really know what happens next, but on my machine the
built-in

screen and the external remains dark. Also the kernel seems to
freeze. I suspect a kernel panic, but I'm not sure. Anyway, the
error

is definitely not well handled, and a fix would be great.
Also, request_firmware() will crash if called for the first time on
the resume path because the file system isn't reachable on the
resume
process. And no cache is available for this firmware. So I guess
that

in this case, request_firmware() returns an error.
Suspend-plug-resume case is not my priority nether as long as it
doesn't make the system crash (Which is currently the case).

On Wed, Oct 12 2022 at 11:25:59 AM +0300, Jani Nikula
wrote:
On Tue, 11 Oct 2022, Matthieu CHARETTE

wrote:
Currently the EDID is requested during the resume. But since
it's
requested too early, this means before the filesystem is
mounted,

the
firmware request fails. This make the DRM driver crash when
resuming.
This kind of issue should be prevented by the firmware caching
process
which cache every firmware requested for the next resume. But
since we
are using a temporary device, the firmware isn't cached on
suspend

since the device doesn't work anymore.
When using a non temporary device to get the EDID, the firmware
will
be cached on suspend for the next resume. So requesting the
firmware
during resume will succeed.
But if the firmware has never been requested since the boot,
this

means that the monitor isn't plugged since the boot. The kernel
will
not be caching the EDID. So if we plug the monitor while the
machine
is suspended. The resume will fail to load the firmware. And the
DRM
driver will crash.
So basically, your fix should solve the issue except for the
case

where the monitor hasn't been plugged since boot and is plugged
while
the machine is suspended.
I hope I was clear. Tell me if I wasn't. I'm not really good at
explaining.

That was a pretty good explanation. The only thing I'm missing is
what
the failure mode is exactly when you claim the driver will crash.
Why
would request_firmware() "crash" if called for the first time on
the

resume path?

I'm not sure I care much about not being able to load the firmware
EDID
in the suspend-plug-resume case (as this can be remedied with a
subsequent modeset), but obviously any errors need to be handled
gracefully, without crashing.

BR,
Jani.

--
Jani Nikula, Intel Open Source Graphics Center

Re: [PATCH 2/2] drm/radeon: Add the missed acpi_put_table() to fix memory leak

Applied.  Thanks

On Fri, Nov 4, 2022 at 6:05 AM Hanjun Guo  wrote:
>
> When the radeon driver reads the bios information from ACPI
> table in radeon_acpi_vfct_bios(), it misses to call acpi_put_table()
> to release the ACPI memory after the init, so add acpi_put_table()
> properly to fix the memory leak.
>
> Fixes: 268ba0a99f89 ("drm/radeon: implement ACPI VFCT vbios fetch (v3)")
> Signed-off-by: Hanjun Guo 
> ---
>  drivers/gpu/drm/radeon/radeon_bios.c | 18 +++---
>  1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_bios.c 
> b/drivers/gpu/drm/radeon/radeon_bios.c
> index 520d1d6..16730c1 100644
> --- a/drivers/gpu/drm/radeon/radeon_bios.c
> +++ b/drivers/gpu/drm/radeon/radeon_bios.c
> @@ -612,13 +612,14 @@ static bool radeon_acpi_vfct_bios(struct radeon_device 
> *rdev)
> acpi_size tbl_size;
> UEFI_ACPI_VFCT *vfct;
> unsigned long offset;
> +   bool r = false;
>
> if (!ACPI_SUCCESS(acpi_get_table("VFCT", 1, &hdr)))
> return false;
> tbl_size = hdr->length;
> if (tbl_size < sizeof(UEFI_ACPI_VFCT)) {
> DRM_ERROR("ACPI VFCT table present but broken (too short 
> #1)\n");
> -   return false;
> +   goto out;
> }
>
> vfct = (UEFI_ACPI_VFCT *)hdr;
> @@ -631,13 +632,13 @@ static bool radeon_acpi_vfct_bios(struct radeon_device 
> *rdev)
> offset += sizeof(VFCT_IMAGE_HEADER);
> if (offset > tbl_size) {
> DRM_ERROR("ACPI VFCT image header truncated\n");
> -   return false;
> +   goto out;
> }
>
> offset += vhdr->ImageLength;
> if (offset > tbl_size) {
> DRM_ERROR("ACPI VFCT image truncated\n");
> -   return false;
> +   goto out;
> }
>
> if (vhdr->ImageLength &&
> @@ -649,15 +650,18 @@ static bool radeon_acpi_vfct_bios(struct radeon_device 
> *rdev)
> rdev->bios = kmemdup(&vbios->VbiosContent,
>  vhdr->ImageLength,
>  GFP_KERNEL);
> +   if (rdev->bios)
> +  r = true;
>
> -   if (!rdev->bios)
> -   return false;
> -   return true;
> +   goto out;
> }
> }
>
> DRM_ERROR("ACPI VFCT table present but broken (too short #2)\n");
> -   return false;
> +
> +out:
> +   acpi_put_table(hdr);
> +   return r;
>  }
>  #else
>  static inline bool radeon_acpi_vfct_bios(struct radeon_device *rdev)
> --
> 1.7.12.4
>

Re: [Intel-gfx] [PATCH v2] drm/i915: Partial abandonment of legacy DRM logging macros

2022-11-08 Thread Andrzej Hajda


On 08.11.2022 13:26, Tvrtko Ursulin wrote:

From: Tvrtko Ursulin 

Convert some usages of legacy DRM logging macros into versions which tell
us on which device have the events occurred.

v2:
  * Don't have struct drm_device as local. (Jani, Ville)

Signed-off-by: Tvrtko Ursulin 
Cc: Jani Nikula 
Cc: John Harrison 
Cc: Ville Syrjälä 


Nice work.

Reviewed-by: Andrzej Hajda 

Regards
Andrzej


---
  drivers/gpu/drm/i915/gem/i915_gem_context.c   |  2 +-
  .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 26 +++
  .../drm/i915/gt/intel_execlists_submission.c  | 13 +++---
  drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c  |  4 +-
  drivers/gpu/drm/i915/gt/intel_gt.c|  4 +-
  drivers/gpu/drm/i915/gt/intel_gt_irq.c|  8 ++--
  drivers/gpu/drm/i915/gt/intel_rps.c   |  6 ++-
  drivers/gpu/drm/i915/gt/intel_workarounds.c   | 43 +++
  .../gpu/drm/i915/gt/intel_workarounds_types.h |  4 ++
  .../gpu/drm/i915/gt/selftest_workarounds.c|  4 +-
  drivers/gpu/drm/i915/i915_debugfs.c   |  4 +-
  drivers/gpu/drm/i915/i915_gem.c   |  2 +-
  drivers/gpu/drm/i915/i915_getparam.c  |  2 +-
  drivers/gpu/drm/i915/i915_irq.c   | 12 +++---
  drivers/gpu/drm/i915/i915_perf.c  | 14 +++---
  drivers/gpu/drm/i915/i915_query.c | 12 +++---
  drivers/gpu/drm/i915/i915_sysfs.c |  3 +-
  drivers/gpu/drm/i915/i915_vma.c   | 16 ---
  drivers/gpu/drm/i915/intel_uncore.c   | 21 +
  19 files changed, 119 insertions(+), 81 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c 
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 01402f3c58f6..7f2831efc798 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -546,7 +546,7 @@ set_proto_ctx_engines_bond(struct i915_user_extension 
__user *base, void *data)
}
  
  	if (intel_engine_uses_guc(master)) {

-   DRM_DEBUG("bonding extension not supported with GuC 
submission");
+   drm_dbg(&i915->drm, "bonding extension not supported with GuC 
submission");
return -ENODEV;
}
  
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c

index 1160723c9d2d..f65fd03f7cf2 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -2148,7 +2148,8 @@ static int eb_move_to_gpu(struct i915_execbuffer *eb)
return err;
  }
  
-static int i915_gem_check_execbuffer(struct drm_i915_gem_execbuffer2 *exec)

+static int i915_gem_check_execbuffer(struct drm_i915_private *i915,
+struct drm_i915_gem_execbuffer2 *exec)
  {
if (exec->flags & __I915_EXEC_ILLEGAL_FLAGS)
return -EINVAL;
@@ -2161,7 +2162,7 @@ static int i915_gem_check_execbuffer(struct 
drm_i915_gem_execbuffer2 *exec)
}
  
  	if (exec->DR4 == 0x) {

-   DRM_DEBUG("UXA submitting garbage DR4, fixing up\n");
+   drm_dbg(&i915->drm, "UXA submitting garbage DR4, fixing up\n");
exec->DR4 = 0;
}
if (exec->DR1 || exec->DR4)
@@ -2799,7 +2800,8 @@ add_timeline_fence_array(struct i915_execbuffer *eb,
  
  		syncobj = drm_syncobj_find(eb->file, user_fence.handle);

if (!syncobj) {
-   DRM_DEBUG("Invalid syncobj handle provided\n");
+   drm_dbg(&eb->i915->drm,
+   "Invalid syncobj handle provided\n");
return -ENOENT;
}
  
@@ -2807,7 +2809,8 @@ add_timeline_fence_array(struct i915_execbuffer *eb,
  
  		if (!fence && user_fence.flags &&

!(user_fence.flags & I915_EXEC_FENCE_SIGNAL)) {
-   DRM_DEBUG("Syncobj handle has no fence\n");
+   drm_dbg(&eb->i915->drm,
+   "Syncobj handle has no fence\n");
drm_syncobj_put(syncobj);
return -EINVAL;
}
@@ -2816,7 +2819,9 @@ add_timeline_fence_array(struct i915_execbuffer *eb,
err = dma_fence_chain_find_seqno(&fence, point);
  
  		if (err && !(user_fence.flags & I915_EXEC_FENCE_SIGNAL)) {

-   DRM_DEBUG("Syncobj handle missing requested point 
%llu\n", point);
+   drm_dbg(&eb->i915->drm,
+   "Syncobj handle missing requested point %llu\n",
+   point);
dma_fence_put(fence);
drm_syncobj_put(syncobj);
return err;
@@ -2842,7 +2847,8 @@ add_timeline_fence_array(struct i915_execbuffer *eb,
 * 0) would break the timeline.
 */
if (user_fence.flags & I915_EXEC_FENCE_WAIT) {
-

Re: [PATCH v2 00/11] Connect VFIO to IOMMUFD

2022-11-08 Thread Yi Liu


On 2022/11/8 17:19, Nicolin Chen wrote:

On Mon, Nov 07, 2022 at 08:52:44PM -0400, Jason Gunthorpe wrote:


This is on github: https://github.com/jgunthorpe/linux/commits/vfio_iommufd

[...]

v2:
  - Rebase to v6.1-rc3, v4 iommufd series
  - Fixup comments and commit messages from list remarks
  - Fix leaking of the iommufd for mdevs
  - New patch to fix vfio modaliases when vfio container is disabled
  - Add a dmesg once when the iommufd provided /dev/vfio/vfio is opened
to signal that iommufd is providing this


I've redone my previous sanity tests. Except those reported bugs,
things look fine. Once we fix those issues, GVT and other modules
can run some more stressful tests, I think.


our side is also starting test (gvt, nic passthrough) this version. need to
wait a while for the result.

--
Regards,
Yi Liu

[PATCH] drm/qxl: Fix missing free_irq

2022-11-08 Thread Wei Li

When doing "cat /proc/interrupts" after qxl.ko is unloaded, an oops occurs:

BUG: unable to handle page fault for address: c0274769
PGD 2a0d067 P4D 2a0d067 PUD 2a0f067 PMD 103f39067 PTE 0
Oops:  [#1] PREEMPT SMP PTI
CPU: 6 PID: 246 Comm: cat Not tainted 6.1.0-rc2 #24
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 
04/01/2014
RIP: 0010:string_nocheck+0x34/0x50
Code: 66 85 c0 74 3c 83 e8 01 4c 8d 5c 07 01 31 c0 eb 19 49 39 fa 76 03 44 88 
07 48 83 c7
RSP: 0018:c9893bb8 EFLAGS: 00010046
RAX:  RBX: c9893c50 RCX: 0a00ff04
RDX: c0274769 RSI: 888102812000 RDI: 88810281133e
RBP: 888102812000 R08: 823fa5e6 R09: 0007
R10: 888102812000 R11: 88820281133d R12: c0274769
R13: 0a00ff04 R14: 0cc4 R15: 823276b4
FS:  0214f8c0() GS:88842fd8() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: c0274769 CR3: 0001025c4005 CR4: 00770ee0
DR0:  DR1:  DR2: 
DR3:  DR6: fffe0ff0 DR7: 0400
PKRU: 5554
Call Trace:
 
 string+0x46/0x60
 vsnprintf+0x27a/0x4f0
 seq_vprintf+0x34/0x50
 seq_printf+0x53/0x70
 ? seq_read_iter+0x365/0x450
 show_interrupts+0x259/0x330
 seq_read_iter+0x2a3/0x450
 proc_reg_read_iter+0x47/0x70
 generic_file_splice_read+0x94/0x160
 splice_direct_to_actor+0xb0/0x230
 ? do_splice_direct+0xd0/0xd0
 do_splice_direct+0x8b/0xd0
 do_sendfile+0x345/0x4f0
 __x64_sys_sendfile64+0xa1/0xc0
 do_syscall_64+0x38/0x90
 entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x4bb0ce
Code: c3 0f 1f 00 4c 89 d2 4c 89 c6 e9 bd fd ff ff 0f 1f 44 00 00 31 c0 c3 0f 
1f 44 00 00
RSP: 002b:7ffd99dc3fb8 EFLAGS: 0246 ORIG_RAX: 0028
RAX: ffda RBX: 0100 RCX: 004bb0ce
RDX:  RSI: 0003 RDI: 0001
RBP: 0001 R08: 0068f240 R09: 0100
R10: 0100 R11: 0246 R12: 0003
R13: 0001 R14:  R15: 
 

It seems that qxl doesn't free the interrupt it requests during unload,
fix this by adding the missing free_irq().

Fixes: f64122c1f6ad ("drm: add new QXL driver. (v1.4)")
Signed-off-by: Wei Li 
---
 drivers/gpu/drm/qxl/qxl_kms.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index dc3828db1991..d591084824de 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -283,6 +283,8 @@ int qxl_device_init(struct qxl_device *qdev,
 void qxl_device_fini(struct qxl_device *qdev)
 {
int cur_idx;
+   struct drm_device *ddev = &qdev->ddev;
+   struct pci_dev *pdev = to_pci_dev(ddev->dev);
 
/* check if qxl_device_init() was successful (gc_work is initialized 
last) */
if (!qdev->gc_work.func)
@@ -305,6 +307,7 @@ void qxl_device_fini(struct qxl_device *qdev)
wait_event_timeout(qdev->release_event,
   atomic_read(&qdev->release_count) == 0,
   HZ);
+   free_irq(pdev->irq, ddev);
flush_work(&qdev->gc_work);
qxl_surf_evict(qdev);
qxl_vram_evict(qdev);
-- 
2.25.1

Re: [PATCH] drm/fb-helper: Fix missing kerneldoc include

2022-11-08 Thread Thomas Zimmermann


Thanks. Merged with fixes for the following warnings.

66ba1150c07e (HEAD -> drm-misc-next) drm/fb-helper: Fix missing 
kerneldoc include
-:6: ERROR:GIT_COMMIT_ID: Please use git commit description style 
'commit <12+ chars of sha1> ("")' - ie: 'commit 8ab59da26bc0 
("drm/fb-helper: Move generic fbdev emulation into separate source file")'

#6:
This was lost in the code movement done in 8ab59da26bc0

-:30: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address 
mismatch: 'From: Daniel Vetter ' != 
'Signed-off-by: Daniel Vetter '


Best regards
Thomas

Am 07.11.22 um 20:09 schrieb Daniel Vetter:

This was lost in the code movement done in 8ab59da26bc0
("drm/fb-helper: Move generic fbdev emulation into separate source
file").

Fixes: 8ab59da26bc0 ("drm/fb-helper: Move generic fbdev emulation into separate 
source file")
Cc: Thomas Zimmermann 
Cc: Javier Martinez Canillas 
Signed-off-by: Daniel Vetter 
---
  Documentation/gpu/drm-kms-helpers.rst | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/Documentation/gpu/drm-kms-helpers.rst 
b/Documentation/gpu/drm-kms-helpers.rst
index dbc85fd7a971..a4860ffd6e86 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -116,6 +116,9 @@ fbdev Helper Functions Reference
  .. kernel-doc:: drivers/gpu/drm/drm_fb_helper.c
 :export:
  
+.. kernel-doc:: drivers/gpu/drm/drm_fbdev_generic.c

+   :export:
+
  format Helper Functions Reference
  =
  


--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH 0/3] drm/fb-helper: Build fixes for cleanup

2022-11-08 Thread Thomas Zimmermann


a-b: danvet via irc

Am 07.11.22 um 13:53 schrieb Thomas Zimmermann:

Cleaning up the fbdev code produced a number of simple build
errors. Fix them.

Thomas Zimmermann (3):
   drm/fbdev: Include 
   drm/hisilicon/hibmc: Include  for readl() and writel()
   drm/fb-helper: Document struct drm_fb_helper.hint_leak_smem_start

  drivers/gpu/drm/drm_fbdev_generic.c  | 1 +
  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 2 ++
  include/drm/drm_fb_helper.h  | 7 +++
  3 files changed, 10 insertions(+)



--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH v3 4/4] drm: panel: Add Jadard JD9365DA-H3 DSI panel

On Tue, Nov 8, 2022 at 3:53 PM Jagan Teki  wrote:
> On Tue, 8 Nov 2022 at 20:18, Linus Walleij  wrote:
> > On Tue, Nov 8, 2022 at 3:12 PM Jagan Teki  wrote:
> > > On Tue, 8 Nov 2022 at 19:31, Linus Walleij  
> > > wrote:
> > > >
> > > > On Thu, Nov 3, 2022 at 3:12 PM Jagan Teki  wrote:
> > > >
> > > > > Jadard JD9365DA-H3 is WXGA MIPI DSI panel and it support TFT
> > > > > dot matrix LCD with 800RGBx1280 dots at maximum.
> > > > >
> > > > > Add support for it.
> > > > >
> > > > > Cc: dri-devel@lists.freedesktop.org
> > > > > Signed-off-by: Jagan Teki 
> > > > > ---
> > > > > Changes for v3:
> > > > > - updatd to WXGA
> > > > > - use JD9365DA_CMD_DCS and JD9365DA_CMD_DELAY
> > > >
> > > > My comments on v2 have not been addressed, for example I asked to
> > > > remove the delay from sequences and just use an explicit delay and
> > > > to then use the existing sequence sending macro.
> > >
> > > True, I responded on the same day [1], since I didn't get the reply I
> > > have posted by assuming my comment is valid. Would you please check
> > > and respond?
> > >
> > > [1] 
> > > https://lore.kernel.org/all/CA+VMnFz0w-6O=wt3iujo1bhqgpz2xbpx6jddz6vg_jw9nht...@mail.gmail.com/
> >
> > OK I see, sorry for not reading close.
> >
> > The driver just supports one single variant.
> >
> > What you are doing is preparing the ground for more variants
> > that may or may not exist. This creates the antipattern "big upfront design"
> > i.e. abstractions added for things that do not yet exist.
> >
> > I think it is better to strip it down to just open coding the delay after
> > the init sequence. When the next variant appears, if ever, they can
> > add abstraction. Maybe they need the same delay in the same
> > place? Who knows...
>
> I understand your point, but delays are strictly mentioned by the
> panel vendor init sequence, cz101b4001 do you think adding in the
> generic or common code is still valid since we have code added for
> cz101b4001 specifically via driver data?

I would instead of encoding a sequence into the driver data
encode a per-variant callback that sends the sequence (which
can then be just static array) and then ends with an explicit delay.

Yours,
Linus Walleij

Re: [PATCH v3 4/4] drm: panel: Add Jadard JD9365DA-H3 DSI panel

On Tue, 8 Nov 2022 at 20:18, Linus Walleij  wrote:
>
> On Tue, Nov 8, 2022 at 3:12 PM Jagan Teki  wrote:
> > On Tue, 8 Nov 2022 at 19:31, Linus Walleij  wrote:
> > >
> > > On Thu, Nov 3, 2022 at 3:12 PM Jagan Teki  wrote:
> > >
> > > > Jadard JD9365DA-H3 is WXGA MIPI DSI panel and it support TFT
> > > > dot matrix LCD with 800RGBx1280 dots at maximum.
> > > >
> > > > Add support for it.
> > > >
> > > > Cc: dri-devel@lists.freedesktop.org
> > > > Signed-off-by: Jagan Teki 
> > > > ---
> > > > Changes for v3:
> > > > - updatd to WXGA
> > > > - use JD9365DA_CMD_DCS and JD9365DA_CMD_DELAY
> > >
> > > My comments on v2 have not been addressed, for example I asked to
> > > remove the delay from sequences and just use an explicit delay and
> > > to then use the existing sequence sending macro.
> >
> > True, I responded on the same day [1], since I didn't get the reply I
> > have posted by assuming my comment is valid. Would you please check
> > and respond?
> >
> > [1] 
> > https://lore.kernel.org/all/CA+VMnFz0w-6O=wt3iujo1bhqgpz2xbpx6jddz6vg_jw9nht...@mail.gmail.com/
>
> OK I see, sorry for not reading close.
>
> The driver just supports one single variant.
>
> What you are doing is preparing the ground for more variants
> that may or may not exist. This creates the antipattern "big upfront design"
> i.e. abstractions added for things that do not yet exist.
>
> I think it is better to strip it down to just open coding the delay after
> the init sequence. When the next variant appears, if ever, they can
> add abstraction. Maybe they need the same delay in the same
> place? Who knows...

I understand your point, but delays are strictly mentioned by the
panel vendor init sequence, cz101b4001 do you think adding in the
generic or common code is still valid since we have code added for
cz101b4001 specifically via driver data?

Thanks,
Jagan.

Re: [PATCH] drm/imx: ipuv3-plane: Fix overlay plane width

2022-11-08 Thread Sebastian Reichel

Hi,

On Tue, Nov 08, 2022 at 03:14:20PM +0100, Philipp Zabel wrote:
> ipu_src_rect_width() was introduced to support odd screen resolutions
> such as 1366x768 by internally rounding up primary plane width to a
> multiple of 8 and compensating with reduced horizontal blanking.
> This also caused overlay plane width to be rounded up, which was not
> intended. Fix overlay plane width by limiting the rounding up to the
> primary plane.
> 
> drm_rect_width(&new_state->src) >> 16 is the same value as
> drm_rect_width(dst) because there is no plane scaling support.
> 
> Fixes: 94dfec48fca7 ("drm/imx: Add 8 pixel alignment fix")
> Signed-off-by: Philipp Zabel 
> ---

Looks sensible, but I no longer have access to the affected
hardware. Maybe Ian or Kitty (both added to Cc) can give it
a test on arch/arm/boot/dts/imx6dl-b155v2.dts

-- Sebastian

>  drivers/gpu/drm/imx/ipuv3-plane.c | 14 --
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c 
> b/drivers/gpu/drm/imx/ipuv3-plane.c
> index dba4f7d81d69..80142d9a4a55 100644
> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
> @@ -614,6 +614,11 @@ static void ipu_plane_atomic_update(struct drm_plane 
> *plane,
>   break;
>   }
>  
> + if (ipu_plane->dp_flow == IPU_DP_FLOW_SYNC_BG)
> + width = ipu_src_rect_width(new_state);
> + else
> + width = drm_rect_width(&new_state->src) >> 16;
> +
>   eba = drm_plane_state_to_eba(new_state, 0);
>  
>   /*
> @@ -622,8 +627,7 @@ static void ipu_plane_atomic_update(struct drm_plane 
> *plane,
>*/
>   if (ipu_state->use_pre) {
>   axi_id = ipu_chan_assign_axi_id(ipu_plane->dma);
> - ipu_prg_channel_configure(ipu_plane->ipu_ch, axi_id,
> -   ipu_src_rect_width(new_state),
> + ipu_prg_channel_configure(ipu_plane->ipu_ch, axi_id, width,
> drm_rect_height(&new_state->src) >> 
> 16,
> fb->pitches[0], fb->format->format,
> fb->modifier, &eba);
> @@ -678,9 +682,8 @@ static void ipu_plane_atomic_update(struct drm_plane 
> *plane,
>   break;
>   }
>  
> - ipu_dmfc_config_wait4eot(ipu_plane->dmfc, ALIGN(drm_rect_width(dst), 
> 8));
> + ipu_dmfc_config_wait4eot(ipu_plane->dmfc, width);
>  
> - width = ipu_src_rect_width(new_state);
>   height = drm_rect_height(&new_state->src) >> 16;
>   info = drm_format_info(fb->format->format);
>   ipu_calculate_bursts(width, info->cpp[0], fb->pitches[0],
> @@ -744,8 +747,7 @@ static void ipu_plane_atomic_update(struct drm_plane 
> *plane,
>   ipu_cpmem_set_burstsize(ipu_plane->ipu_ch, 16);
>  
>   ipu_cpmem_zero(ipu_plane->alpha_ch);
> - ipu_cpmem_set_resolution(ipu_plane->alpha_ch,
> -  ipu_src_rect_width(new_state),
> + ipu_cpmem_set_resolution(ipu_plane->alpha_ch, width,
>drm_rect_height(&new_state->src) >> 
> 16);
>   ipu_cpmem_set_format_passthrough(ipu_plane->alpha_ch, 8);
>   ipu_cpmem_set_high_priority(ipu_plane->alpha_ch);
> 
> base-commit: 9abf2313adc1ca1b6180c508c25f22f9395cc780
> -- 
> 2.30.2
> 


signature.asc
Description: PGP signature

Re: [PATCH v3 4/4] drm: panel: Add Jadard JD9365DA-H3 DSI panel

On Tue, Nov 8, 2022 at 3:12 PM Jagan Teki  wrote:
> On Tue, 8 Nov 2022 at 19:31, Linus Walleij  wrote:
> >
> > On Thu, Nov 3, 2022 at 3:12 PM Jagan Teki  wrote:
> >
> > > Jadard JD9365DA-H3 is WXGA MIPI DSI panel and it support TFT
> > > dot matrix LCD with 800RGBx1280 dots at maximum.
> > >
> > > Add support for it.
> > >
> > > Cc: dri-devel@lists.freedesktop.org
> > > Signed-off-by: Jagan Teki 
> > > ---
> > > Changes for v3:
> > > - updatd to WXGA
> > > - use JD9365DA_CMD_DCS and JD9365DA_CMD_DELAY
> >
> > My comments on v2 have not been addressed, for example I asked to
> > remove the delay from sequences and just use an explicit delay and
> > to then use the existing sequence sending macro.
>
> True, I responded on the same day [1], since I didn't get the reply I
> have posted by assuming my comment is valid. Would you please check
> and respond?
>
> [1] 
> https://lore.kernel.org/all/CA+VMnFz0w-6O=wt3iujo1bhqgpz2xbpx6jddz6vg_jw9nht...@mail.gmail.com/

OK I see, sorry for not reading close.

The driver just supports one single variant.

What you are doing is preparing the ground for more variants
that may or may not exist. This creates the antipattern "big upfront design"
i.e. abstractions added for things that do not yet exist.

I think it is better to strip it down to just open coding the delay after
the init sequence. When the next variant appears, if ever, they can
add abstraction. Maybe they need the same delay in the same
place? Who knows...

Yours,
Linus Walleij

Re: [PATCH v2 03/18] dt-bindings: msm: dsi-controller-main: Add vdd* descriptions back in

2022-11-08 Thread Bryan O'Donoghue


On 08/11/2022 12:59, Dmitry Baryshkov wrote:


Warnings about missing regulators can be resolved by updating the 
relevant

dtsi files to point to fixed always-on regulators where appropriate.


Ugh. Are they missing or are they optional/not used on these platforms?


Some platforms either don't implement them or worse possibly do 
implement but don't model them when they should.



Can you possibly list all regulator warnings?



Downstream we have

arch/arm/boot/dts/qcom/msm8916-mdss.dtsi

mdss_dsi0: qcom,mdss_dsi@1a98000 {
vdda-supply = <&pm8916_l2>;
vdd-supply = <&pm8916_l17>;
vddio-supply = <&pm8916_l6>;
};

Looking at something like

arch/arm/boot/dts/qcom/msm8916-mtp.dtsi which references 
arch/arm/boot/dts/qcom/dsi-panel-jdi-1080p-video.dtsi it doesn't appear 
to delete andy of the vdd*-supply references


apq8016-sbc.dtb: dsi@1a98000: 'vdd-supply' is a required property
msm8916-samsung-a3u-eur.dtb: dsi@1a98000: 'vdd-supply' is a required 
property
msm8916-samsung-a5u-eur.dtb: dsi@1a98000: 'vdd-supply' is a required 
property

msm8916-samsung-e5.dtb: dsi@1a98000: 'vdd-supply' is a required property
msm8916-samsung-e7.dtb: dsi@1a98000: 'vdd-supply' is a required property
msm8916-samsung-grandmax.dtb: dsi@1a98000: 'vdd-supply' is a required 
property
msm8996-xiaomi-natrium.dtb: dsi@994000: 'panel@0', 'vcca-supply' do not 
match any of the regexes: 'pinctrl-[0-9]+'

msm8996-xiaomi-scorpio.dtb: dsi@994000: 'vdda-supply' is a required property
qrb5165-rb5.dtb: dsi@ae94000: 'vdd-supply' is a required property
qrb5165-rb5.dtb: dsi@ae94000: 'vddio-supply' is a required property
sc7180-idp.dtb: dsi@ae94000: 'vdd-supply' is a required property
sc7180-idp.dtb: dsi@ae94000: 'vddio-supply' is a required property
sc7180-trogdor-coachz-r1.dtb: dsi@ae94000: 'vdd-supply' is a required 
property
sc7180-trogdor-coachz-r1.dtb: dsi@ae94000: 'vddio-supply' is a required 
property
sc7180-trogdor-coachz-r1-lte.dtb: dsi@ae94000: 'vdd-supply' is a 
required property
sc7180-trogdor-coachz-r1-lte.dtb: dsi@ae94000: 'vddio-supply' is a 
required property
sc7180-trogdor-coachz-r3.dtb: dsi@ae94000: 'vdd-supply' is a required 
property
sc7180-trogdor-coachz-r3.dtb: dsi@ae94000: 'vddio-supply' is a required 
property
sc7180-trogdor-coachz-r3-lte.dtb: dsi@ae94000: 'vdd-supply' is a 
required property
sc7180-trogdor-coachz-r3-lte.dtb: dsi@ae94000: 'vddio-supply' is a 
required property
sc7180-trogdor-homestar-r2.dtb: dsi@ae94000: 'vdd-supply' is a required 
property
sc7180-trogdor-homestar-r2.dtb: dsi@ae94000: 'vddio-supply' is a 
required property
sc7180-trogdor-homestar-r3.dtb: dsi@ae94000: 'vdd-supply' is a required 
property
sc7180-trogdor-homestar-r3.dtb: dsi@ae94000: 'vddio-supply' is a 
required property
sc7180-trogdor-homestar-r4.dtb: dsi@ae94000: 'vdd-supply' is a required 
property
sc7180-trogdor-homestar-r4.dtb: dsi@ae94000: 'vddio-supply' is a 
required property
sc7180-trogdor-kingoftown-r0.dtb: dsi@ae94000: 'vdd-supply' is a 
required property
sc7180-trogdor-kingoftown-r0.dtb: dsi@ae94000: 'vddio-supply' is a 
required property
sc7180-trogdor-kingoftown-r1.dtb: dsi@ae94000: 'vdd-supply' is a 
required property
sc7180-trogdor-kingoftown-r1.dtb: dsi@ae94000: 'vddio-supply' is a 
required property
sc7180-trogdor-lazor-r0.dtb: dsi@ae94000: 'vdd-supply' is a required 
property
sc7180-trogdor-lazor-r0.dtb: dsi@ae94000: 'vddio-supply' is a required 
property
sc7180-trogdor-lazor-r1.dtb: dsi@ae94000: 'vdd-supply' is a required 
property
sc7180-trogdor-lazor-r1.dtb: dsi@ae94000: 'vddio-supply' is a required 
property
sc7180-trogdor-lazor-r1-kb.dtb: dsi@ae94000: 'vdd-supply' is a required 
property
sc7180-trogdor-lazor-r1-kb.dtb: dsi@ae94000: 'vddio-supply' is a 
required property
sc7180-trogdor-lazor-r1-lte.dtb: dsi@ae94000: 'vdd-supply' is a required 
property
sc7180-trogdor-lazor-r1-lte.dtb: dsi@ae94000: 'vddio-supply' is a 
required property
sc7180-trogdor-lazor-r3.dtb: dsi@ae94000: 'vdd-supply' is a required 
property
sc7180-trogdor-lazor-r3.dtb: dsi@ae94000: 'vddio-supply' is a required 
property
sc7180-trogdor-lazor-r3-kb.dtb: dsi@ae94000: 'vdd-supply' is a required 
property
sc7180-trogdor-lazor-r3-kb.dtb: dsi@ae94000: 'vddio-supply' is a 
required property
sc7180-trogdor-lazor-r3-lte.dtb: dsi@ae94000: 'vdd-supply' is a required 
property
sc7180-trogdor-lazor-r3-lte.dtb: dsi@ae94000: 'vddio-supply' is a 
required property
sc7180-trogdor-lazor-r9.dtb: dsi@ae94000: 'vdd-supply' is a required 
property
sc7180-trogdor-lazor-r9.dtb: dsi@ae94000: 'vddio-supply' is a required 
property
sc7180-trogdor-lazor-r9-kb.dtb: dsi@ae94000: 'vdd-supply' is a required 
property
sc7180-trogdor-lazor-r9-kb.dtb: dsi@ae94000: 'vddio-supply' is a 
required property
sc7180-trogdor-lazor-r9-lte.dtb: dsi@ae94000: 'vdd-supply' is a required 
property
sc7180-trogdor-lazor-r9-lte.dtb: dsi@ae94000: 'vddio-supply' is a 
required property
sc7180-trogdor-lazor-limozeen-r4.dtb: dsi@ae94000: 'vdd-supply' is a 
required property
sc718

[PATCH] drm/imx: ipuv3-plane: Fix overlay plane width

2022-11-08 Thread Philipp Zabel

ipu_src_rect_width() was introduced to support odd screen resolutions
such as 1366x768 by internally rounding up primary plane width to a
multiple of 8 and compensating with reduced horizontal blanking.
This also caused overlay plane width to be rounded up, which was not
intended. Fix overlay plane width by limiting the rounding up to the
primary plane.

drm_rect_width(&new_state->src) >> 16 is the same value as
drm_rect_width(dst) because there is no plane scaling support.

Fixes: 94dfec48fca7 ("drm/imx: Add 8 pixel alignment fix")
Signed-off-by: Philipp Zabel 
---
 drivers/gpu/drm/imx/ipuv3-plane.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c 
b/drivers/gpu/drm/imx/ipuv3-plane.c
index dba4f7d81d69..80142d9a4a55 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -614,6 +614,11 @@ static void ipu_plane_atomic_update(struct drm_plane 
*plane,
break;
}
 
+   if (ipu_plane->dp_flow == IPU_DP_FLOW_SYNC_BG)
+   width = ipu_src_rect_width(new_state);
+   else
+   width = drm_rect_width(&new_state->src) >> 16;
+
eba = drm_plane_state_to_eba(new_state, 0);
 
/*
@@ -622,8 +627,7 @@ static void ipu_plane_atomic_update(struct drm_plane *plane,
 */
if (ipu_state->use_pre) {
axi_id = ipu_chan_assign_axi_id(ipu_plane->dma);
-   ipu_prg_channel_configure(ipu_plane->ipu_ch, axi_id,
- ipu_src_rect_width(new_state),
+   ipu_prg_channel_configure(ipu_plane->ipu_ch, axi_id, width,
  drm_rect_height(&new_state->src) >> 
16,
  fb->pitches[0], fb->format->format,
  fb->modifier, &eba);
@@ -678,9 +682,8 @@ static void ipu_plane_atomic_update(struct drm_plane *plane,
break;
}
 
-   ipu_dmfc_config_wait4eot(ipu_plane->dmfc, ALIGN(drm_rect_width(dst), 
8));
+   ipu_dmfc_config_wait4eot(ipu_plane->dmfc, width);
 
-   width = ipu_src_rect_width(new_state);
height = drm_rect_height(&new_state->src) >> 16;
info = drm_format_info(fb->format->format);
ipu_calculate_bursts(width, info->cpp[0], fb->pitches[0],
@@ -744,8 +747,7 @@ static void ipu_plane_atomic_update(struct drm_plane *plane,
ipu_cpmem_set_burstsize(ipu_plane->ipu_ch, 16);
 
ipu_cpmem_zero(ipu_plane->alpha_ch);
-   ipu_cpmem_set_resolution(ipu_plane->alpha_ch,
-ipu_src_rect_width(new_state),
+   ipu_cpmem_set_resolution(ipu_plane->alpha_ch, width,
 drm_rect_height(&new_state->src) >> 
16);
ipu_cpmem_set_format_passthrough(ipu_plane->alpha_ch, 8);
ipu_cpmem_set_high_priority(ipu_plane->alpha_ch);

base-commit: 9abf2313adc1ca1b6180c508c25f22f9395cc780
-- 
2.30.2

Re: [PATCH v3 4/4] drm: panel: Add Jadard JD9365DA-H3 DSI panel

On Tue, 8 Nov 2022 at 19:31, Linus Walleij  wrote:
>
> On Thu, Nov 3, 2022 at 3:12 PM Jagan Teki  wrote:
>
> > Jadard JD9365DA-H3 is WXGA MIPI DSI panel and it support TFT
> > dot matrix LCD with 800RGBx1280 dots at maximum.
> >
> > Add support for it.
> >
> > Cc: dri-devel@lists.freedesktop.org
> > Signed-off-by: Jagan Teki 
> > ---
> > Changes for v3:
> > - updatd to WXGA
> > - use JD9365DA_CMD_DCS and JD9365DA_CMD_DELAY
>
> My comments on v2 have not been addressed, for example I asked to
> remove the delay from sequences and just use an explicit delay and
> to then use the existing sequence sending macro.

True, I responded on the same day [1], since I didn't get the reply I
have posted by assuming my comment is valid. Would you please check
and respond?

[1] 
https://lore.kernel.org/all/CA+VMnFz0w-6O=wt3iujo1bhqgpz2xbpx6jddz6vg_jw9nht...@mail.gmail.com/

Thanks,
Jagan.

Re: [PATCH] drm/panfrost: Update io-pgtable API

2022-11-08 Thread Dmitry Osipenko

On 11/7/22 19:50, Robin Murphy wrote:
> On 2022-11-04 20:48, Dmitry Osipenko wrote:
>> On 11/4/22 23:37, Robin Murphy wrote:
>>> On 2022-11-04 20:11, Dmitry Osipenko wrote:
 On 8/23/22 01:01, Robin Murphy wrote:
> Convert to io-pgtable's bulk {map,unmap}_pages() APIs, to help the old
> single-page interfaces eventually go away. Unmapping heap BOs still
> wants to be done a page at a time, but everything else can get the
> full
> benefit of the more efficient interface.
>
> Signed-off-by: Robin Murphy 
> ---
>    drivers/gpu/drm/panfrost/panfrost_mmu.c | 40
> +++--
>    1 file changed, 25 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> index b285a8001b1d..e246d914e7f6 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> @@ -248,11 +248,15 @@ void panfrost_mmu_reset(struct panfrost_device
> *pfdev)
>    mmu_write(pfdev, MMU_INT_MASK, ~0);
>    }
>    -static size_t get_pgsize(u64 addr, size_t size)
> +static size_t get_pgsize(u64 addr, size_t size, size_t *count)
>    {
> -    if (addr & (SZ_2M - 1) || size < SZ_2M)
> -    return SZ_4K;
> +    size_t blk_offset = -addr % SZ_2M;
>    +    if (blk_offset || size < SZ_2M) {
> +    *count = min_not_zero(blk_offset, size) / SZ_4K;
> +    return SZ_4K;
> +    }
> +    *count = size / SZ_2M;
>    return SZ_2M;
>    }
>    @@ -287,12 +291,16 @@ static int mmu_map_sg(struct panfrost_device
> *pfdev, struct panfrost_mmu *mmu,
>    dev_dbg(pfdev->dev, "map: as=%d, iova=%llx, paddr=%lx,
> len=%zx", mmu->as, iova, paddr, len);
>      while (len) {
> -    size_t pgsize = get_pgsize(iova | paddr, len);
> +    size_t pgcount, mapped = 0;
> +    size_t pgsize = get_pgsize(iova | paddr, len, &pgcount);
>    -    ops->map(ops, iova, paddr, pgsize, prot, GFP_KERNEL);
> -    iova += pgsize;
> -    paddr += pgsize;
> -    len -= pgsize;
> +    ops->map_pages(ops, iova, paddr, pgsize, pgcount, prot,
> +   GFP_KERNEL, &mapped);
> +    /* Don't get stuck if things have gone wrong */
> +    mapped = max(mapped, pgsize);
> +    iova += mapped;
> +    paddr += mapped;
> +    len -= mapped;
>    }
>    }
>    @@ -344,15 +352,17 @@ void panfrost_mmu_unmap(struct
> panfrost_gem_mapping *mapping)
>    mapping->mmu->as, iova, len);
>      while (unmapped_len < len) {
> -    size_t unmapped_page;
> -    size_t pgsize = get_pgsize(iova, len - unmapped_len);
> +    size_t unmapped_page, pgcount;
> +    size_t pgsize = get_pgsize(iova, len - unmapped_len,
> &pgcount);
>    -    if (ops->iova_to_phys(ops, iova)) {
> -    unmapped_page = ops->unmap(ops, iova, pgsize, NULL);
> -    WARN_ON(unmapped_page != pgsize);
> +    if (bo->is_heap)
> +    pgcount = 1;
> +    if (!bo->is_heap || ops->iova_to_phys(ops, iova)) {
> +    unmapped_page = ops->unmap_pages(ops, iova, pgsize,
> pgcount, NULL);
> +    WARN_ON(unmapped_page != pgsize * pgcount);

 This patch causes this WARN_ON to trigger. It doesn't happen all the
 time, I see that the whole unmapped area is mapped. Initially, I
 thought
 that this happens because it tries to unmap a partially mapped range,
 but I checked that ops->iova_to_phys() returns address for all 4k
 chunks.

 For example the pgsize * pgcount = 0x800, while returned
 unmapped_page = 0x600.

 I don't see this problem with this patch reverted. This is using
 today's
 linux-next. Any ideas?
>>>
>>> What's the base IOVA in such a case? I'm wondering if the truncated size
>>> lines up to any interesting boundary. Presumably you're not seeing any
>>> additional warnings from io-pgtable itself?
>>
>> No warnings from io-pgtable. It succeeds for 0x3200 and fails for
>> 0x3a00 using same size 0x800. It actually fails only for the
>> 0x3a00 as far as I see from my logs. Perhaps it indeed has to do
>> something with the boundary.
> 
> Bleh, indeed even though we don't use 1GB block mappings, we still need
> to split at 1GB boundaries to match what the IOMMU API will do, and thus
> what io-pgtable expects. I guess I hadn't really considered that we
> might ever have that much graphics memory in play at once...
> 
> The fix probably looks like this:
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> index e246d914e7f6..6abc7d3726dd 100644
> --- a/drivers/gpu/drm/panf

Re: [PATCH v3 4/4] drm: panel: Add Jadard JD9365DA-H3 DSI panel

On Thu, Nov 3, 2022 at 3:12 PM Jagan Teki  wrote:

> Jadard JD9365DA-H3 is WXGA MIPI DSI panel and it support TFT
> dot matrix LCD with 800RGBx1280 dots at maximum.
>
> Add support for it.
>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Jagan Teki 
> ---
> Changes for v3:
> - updatd to WXGA
> - use JD9365DA_CMD_DCS and JD9365DA_CMD_DELAY

My comments on v2 have not been addressed, for example I asked to
remove the delay from sequences and just use an explicit delay and
to then use the existing sequence sending macro.

Yours,
Linus Walleij

RE: [EXT] Re: [PATCH v3 00/10] Initial support for Cadence MHDP(HDMI/DP) for i.MX8MQ

2022-11-08 Thread Sandor Yu

Run get_maintainer.pl for patch 1, 
Your email address is list as follow,
Jani Nikula  (commit_signer:2/8=25%)

And I add the email address that comment as "commit_signer" into Cc list.

B.R
Sandor

> -Original Message-
> From: Jani Nikula 
> Sent: 2022年11月8日 21:16
> To: Sandor Yu ; dri-devel@lists.freedesktop.org;
> devicet...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> linux-ker...@vger.kernel.org; linux-...@lists.infradead.org;
> andrzej.ha...@intel.com; neil.armstr...@linaro.org; robert.f...@linaro.org;
> laurent.pinch...@ideasonboard.com; jo...@kwiboo.se;
> jernej.skra...@gmail.com; vk...@kernel.org
> Cc: robh...@kernel.org; krzysztof.kozlowski...@linaro.org;
> shawn...@kernel.org; s.ha...@pengutronix.de; ker...@pengutronix.de;
> dl-linux-imx ; tzimmerm...@suse.de;
> ly...@redhat.com; Sandor Yu ; javi...@redhat.com;
> ville.syrj...@linux.intel.com; s...@ravnborg.org; max...@cerno.tech;
> penguin-ker...@i-love.sakura.ne.jp; Oliver Brown 
> Subject: [EXT] Re: [PATCH v3 00/10] Initial support for Cadence
> MHDP(HDMI/DP) for i.MX8MQ
> 
> Caution: EXT Email
> 
> On Tue, 08 Nov 2022, Sandor Yu  wrote:
> > The patch set initial support for Cadence MHDP(HDMI/DP) DRM bridge
> > drivers and Cadence HDP-TX PHY(HDMI/DP) drivers for iMX8MQ.
> 
> Has some get_maintainer.pl guidance changed recently or something, or why
> am I increasingly being Cc'd on patches that are largely irrelevant to me?
> 
> BR,
> Jani.
> 
> --
> Jani Nikula, Intel Open Source Graphics Center

Re: [Nouveau] [PATCH v7 00/23] drm: Analog TV Improvements

2022-11-08 Thread Geert Uytterhoeven

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: [PATCH v2 35/65] clk: ux500: sysctrl: Add a determine_rate hook

On Fri, Nov 4, 2022 at 2:32 PM Maxime Ripard  wrote:

> The UX500 sysctrl "set_parent" clocks implement a mux with a set_parent
> hook, but doesn't provide a determine_rate implementation.
>
> This is a bit odd, since set_parent() is there to, as its name implies,
> change the parent of a clock. However, the most likely candidate to
> trigger that parent change is a call to clk_set_rate(), with
> determine_rate() figuring out which parent is the best suited for a
> given rate.
>
> The other trigger would be a call to clk_set_parent(), but it's far less
> used, and it doesn't look like there's any obvious user for that clock.
>
> So, the set_parent hook is effectively unused, possibly because of an
> oversight. However, it could also be an explicit decision by the
> original author to avoid any reparenting but through an explicit call to
> clk_set_parent().
>
> The latter case would be equivalent to setting the flag
> CLK_SET_RATE_NO_REPARENT, together with setting our determine_rate hook
> to __clk_mux_determine_rate(). Indeed, if no determine_rate
> implementation is provided, clk_round_rate() (through
> clk_core_round_rate_nolock()) will call itself on the parent if
> CLK_SET_RATE_PARENT is set, and will not change the clock rate
> otherwise. __clk_mux_determine_rate() has the exact same behavior when
> CLK_SET_RATE_NO_REPARENT is set.
>
> And if it was an oversight, then we are at least explicit about our
> behavior now and it can be further refined down the line.
>
> Signed-off-by: Maxime Ripard 

Acked-by: Linus Walleij 

Yours,
Linus Walleij

Re: [PATCH v2 34/65] clk: ux500: prcmu: Add a determine_rate hook