[PATCH 1/2] drm/msm: provide fb_dirty implemenation
Since commit 93e81e38e197 ("drm/fb_helper: Minimize damage-helper
overhead") the drm_fb_helper_funcs::fb_dirty helper is required for
proper dirty/damage processing. The drm/msm driver requires that to
function to let CMD panels to work. Use simplified version of
drm_fbdev_generic_helper_fb_dirty() to fix support for CMD mode panels.
Reported-by: Degdag Mohamed
Fixes: 93e81e38e197 ("drm/fb_helper: Minimize damage-helper overhead")
Cc: Thomas Zimmermann
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/msm_fbdev.c | 20
1 file changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c
index fa9c1cbffae3..b933a85420f6 100644
--- a/drivers/gpu/drm/msm/msm_fbdev.c
+++ b/drivers/gpu/drm/msm/msm_fbdev.c
@@ -139,8 +139,28 @@ static int msm_fbdev_create(struct drm_fb_helper *helper,
return ret;
}
+static int msm_fbdev_fb_dirty(struct drm_fb_helper *helper,
+ struct drm_clip_rect *clip)
+{
+ struct drm_device *dev = helper->dev;
+ int ret;
+
+ /* Call damage handlers only if necessary */
+ if (!(clip->x1 < clip->x2 && clip->y1 < clip->y2))
+ return 0;
+
+ if (helper->fb->funcs->dirty) {
+ ret = helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, clip, 1);
+ if (drm_WARN_ONCE(dev, ret, "Dirty helper failed: ret=%d\n",
ret))
+ return ret;
+ }
+
+ return 0;
+}
+
static const struct drm_fb_helper_funcs msm_fb_helper_funcs = {
.fb_probe = msm_fbdev_create,
+ .fb_dirty = msm_fbdev_fb_dirty,
};
/*
--
2.39.2
[PATCH 2/2] drm/msm/dsi: don't allow enabling 7nm VCO with unprogrammed rate
CCF can try enabling VCO before the rate has been programmed. This can
cause clock lockups and/or other boot issues. Program the VCO to the
minimal PLL rate if the read rate is 0 Hz.
Reported-by: Degdag Mohamed
Fixes: 1ef7c99d145c ("drm/msm/dsi: add support for 7nm DSI PHY/PLL")
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
index 3b1ed02f644d..6979d35eb7c3 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
@@ -395,11 +395,16 @@ static void dsi_pll_phy_dig_reset(struct dsi_pll_7nm *pll)
wmb(); /* Ensure that the reset is deasserted */
}
+static unsigned long dsi_pll_7nm_vco_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate);
static int dsi_pll_7nm_vco_prepare(struct clk_hw *hw)
{
struct dsi_pll_7nm *pll_7nm = to_pll_7nm(hw);
int rc;
+ if (dsi_pll_7nm_vco_recalc_rate(hw, VCO_REF_CLK_RATE) == 0)
+ dsi_pll_7nm_vco_set_rate(hw, pll_7nm->phy->cfg->min_pll_rate,
VCO_REF_CLK_RATE);
+
dsi_pll_enable_pll_bias(pll_7nm);
if (pll_7nm->slave)
dsi_pll_enable_pll_bias(pll_7nm->slave);
--
2.39.2
Re: [PATCH] drm/i2c: Switch i2c drivers back to use .probe()
Hi Uwe,
On Sun, Jun 11, 2023 at 10:27:40PM +0200, Uwe Kleine-König wrote:
> After commit b8a1a4cd5a98 ("i2c: Provide a temporary .probe_new()
> call-back type"), all drivers being converted to .probe_new() and then
> commit 03c835f498b5 ("i2c: Switch .probe() to not take an id parameter")
> convert back to (the new) .probe() to be able to eventually drop
> .probe_new() from struct i2c_driver.
>
> Signed-off-by: Uwe Kleine-König
Reviewed-by: Andi Shyti
Andi
Re: [PATCH v6 6/6] drm/msm/dsi: Document DSC related pclk_rate and hdisplay calculations
On 2023-06-09 15:57:18, Jessica Zhang wrote:
> Add documentation comments explaining the pclk_rate and hdisplay math
> related to DSC.
>
> Signed-off-by: Jessica Zhang
> ---
> drivers/gpu/drm/msm/dsi/dsi_host.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index fb1d3a25765f..aeaadc18bc7b 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -564,6 +564,13 @@ void dsi_link_clk_disable_v2(struct msm_dsi_host
> *msm_host)
> static unsigned long dsi_adjust_pclk_for_compression(const struct
> drm_display_mode *mode,
> const struct drm_dsc_config *dsc)
> {
> + /*
> + * Adjust the pclk rate by calculating a new hdisplay proportional to
> + * the compression ratio such that:
> + * new_hdisplay = old_hdisplay * target_bpp / source_bpp
> + *
> + * Porches need not be adjusted during compression.
> + */
> int new_hdisplay = DIV_ROUND_UP(mode->hdisplay *
> drm_dsc_get_bpp_int(dsc),
> dsc->bits_per_component * 3);
I won't reiterate my original troubles with this logic and the comment
as that has well been described in v5 replies.
Just want to ask why this comment couldn't be added in patch 5/6
immediately when the logic is introduced? Now readers won't have a clue
what is going on until they skip one patch ahead.
Furthermore it is lacking any explanation that this is a workaround for
cmd-mode, and that porches are currently used to represent "transfer
time" until those calculations are implemented. At that point there is
no concept of "not adjusting porches for compressed signals" anymore.
>
> @@ -961,6 +968,9 @@ static void dsi_timing_setup(struct msm_dsi_host
> *msm_host, bool is_bonded_dsi)
>
> /* Divide the display by 3 but keep back/font porch and
>* pulse width same
> + *
> + * hdisplay will be divided by 3 here to account for the fact
> + * that DPU sends 3 bytes per pclk cycle to DSI.
>*/
> h_total -= hdisplay;
> hdisplay =
> DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 3);
Still very glad to have this, thank you for adding it. Note that it
only further undermines the pclk adjustments, as I just explained in v5
review.
- Marijn
>
> --
> 2.40.1
>
Re: [Freedreno] [PATCH v5 2/5] drm/msm/dsi: Adjust pclk rate for compression
On 2023-06-08 17:56:47, Jessica Zhang wrote:
> > As discussed before we realized that this change is more-or-less a hack,
> > since downstream calculates pclk quite differently - at least for
> > command-mode panels. Do you still intend to land this patch this way,
> > or go the proper route by introducing the right math from the get-go?
> > Or is the math at least correct for video-mode panels?
>
> Sorry but can you please clarify what exactly is incorrect or different
> about this math when compared to downstream? And, if you think that this
> math is incorrect, what exactly has to be changed to make it the "right
> math"?
>
> We've already shown step-by-step [1] not only how the resulting pclk
> from the downstream code matches out upstream calculations, but also how
> the inclusion of porches in the upstream math would make up for the fact
> that upstream has no concept of transfer time [2].
But it doesn't match, unless one hardcodes the desired clock (and/or
tranfer_time calculations) in a panel driver and uses that to come up
with artificial porches in the DRM mode.
> If the lack of transfer time in the upstream math is the issue, I
> believe that that's not within the scope of this series, as transfer
> time is not something specific to DSC.
Yes, that is the issue, and there is zero documentation in this patch
describing that it is currently a workaround to rescale the hdisplay
portion.
After all, when there are no porches pretending to make up for the lack
of transfer time, this code will be obsolete.
> Moreover, as stated in an earlier revision [3], there is no way to
> validate DSC over DSI for video mode. As far as I know, we do not have a
> way to validate video mode datapath for DSI in general.
It was just a question wheter a calculation of this form is correct for
video mode, where porches are used and not - afaik - tranfer time?
> [1] https://gitlab.freedesktop.org/drm/msm/-/issues/24#note_1936144
> [2] https://gitlab.freedesktop.org/drm/msm/-/issues/24#note_1945792
> [3]
> https://patchwork.freedesktop.org/patch/535117/?series=117219=1#comment_970492
>
> >
> > This function requires a documentation comment to explain that all.
> >
> >> + const struct drm_dsc_config *dsc)
> >> +{
> >> + int new_hdisplay = DIV_ROUND_UP(mode->hdisplay *
> >> drm_dsc_get_bpp_int(dsc),
> >
> > This sounds like a prime candidate for msm_dsc_get_bytes_per_line(), if
> > bits_per_component==8 is assumed. In fact, it then becomes identical
> > to the following line in dsi_host.c which you added previously:
> >
> > hdisplay = DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 3);
> >
> > If not, what is the difference between these two calculations? Maybe
> > they both need to be in a properly-named helper.
>
> While the math technically matches up (assuming, also, that
> mode->hdisplay == slice_width * slice_count for all cases), there are
> conceptual differences between the pclk and hdisplay calculations.
>
> Just to reiterate what was already said on IRC:
>
> In the pclk calculation, we're multiplying pclk by the compression ratio
> (which would be target_bpp / src_bpp) -- please note that here, we
> calculate src_bpp by doing bpc * 3.
>
> In the hdisplay calculation, we calculate the bytes per line and divide
> by 3 (bytes) to account for the fact that we can only process 3 bytes
> per pclk cycle.
Your comment in v6 explains that hdisplay is divided by 3 because "DPU
sends 3 bytes per pclk cycle to DSI". That inherently describes **a
relation between hdisplay and pclk** so why is the math then different?
After all, pclk is calculated based on the number of bytes (after DSC
compression) that need to be sent from DPU to DSI... and that has
nothing to do with the number of source bytes.
Note that the original hdisplay neither has any notion of bytes: it is
the _number of horizontal pixels_.
> So while I understand why you would want to put this behind a shared
> helper, I think abstracting the pclk and hdisplay math away would
> obfuscate the conceptual difference between the 2 calculations.
That difference is still unclear, FWIW.
> >> + dsc->bits_per_component * 3);
And bits_per_component hasn't been used before yet... It is the number
of bits in the original image, so this could just be dsi_get_bpp() as
used elsewhere?
> >
> > As we established in the drm/msm issue [2] there is currently a
> > confusion whether this /3 (and the /3 in dsi_timing_setup) come from the
> > ratio between dsi_get_bpp() and dsc->bpp or something else. Can you
> > clarify that with constants and comments?
>
> Sure, we are planning to add a patch to the end of this series
> documenting the math.
Why can't you - at least for the new math introduced right here -
document it right from the get-go? Having a separate patch to add it
seems extraneous; though extra documentation for existing code is always
welcome.
- Marijn
Re: [PATCH v5 2/5] drm/msm/dsi: Adjust pclk rate for compression
On 2023-06-09 19:58:00, Dmitry Baryshkov wrote:
> On 08/06/2023 23:36, Marijn Suijten wrote:
> > Same title suggestion as earlier: s/adjust/reduce
> >
> > On 2023-05-22 18:08:56, Jessica Zhang wrote:
> >> Adjust the pclk rate to divide hdisplay by the compression ratio when DSC
> >> is enabled.
> >>
> >> Signed-off-by: Jessica Zhang
> >> ---
> >> drivers/gpu/drm/msm/dsi/dsi_host.c | 21 ++---
> >> 1 file changed, 18 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> >> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> >> index a448931af804..88f370dd2ea1 100644
> >> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> >> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> >> @@ -561,7 +561,18 @@ void dsi_link_clk_disable_v2(struct msm_dsi_host
> >> *msm_host)
> >>clk_disable_unprepare(msm_host->byte_clk);
> >> }
> >>
> >> -static unsigned long dsi_get_pclk_rate(const struct drm_display_mode
> >> *mode, bool is_bonded_dsi)
> >> +static unsigned long dsi_adjust_compressed_pclk(const struct
> >> drm_display_mode *mode,
> >
> > Nit: adjust_pclk_for_compression
> >
> > As discussed before we realized that this change is more-or-less a hack,
> > since downstream calculates pclk quite differently - at least for
> > command-mode panels. Do you still intend to land this patch this way,
> > or go the proper route by introducing the right math from the get-go?
> > Or is the math at least correct for video-mode panels?
>
> Can we please postpone the cmd-vs-video discussion?
If you had read Jessica's reply (and the discussions thus far) this
patch is intended for CMD-mode:
Moreover, as stated in an earlier revision [3], there is no way to
validate DSC over DSI for video mode. As far as I know, we do not
have a way to validate video mode datapath for DSI in general.
Hence my hopefully-relevant question whether this workaround - to reduce
the hdisplay portion that comprises the full pclk signal - is at the
very least right for video mode? After all, CMD mode doesn't have
porches so it makes no sense to account for those (except that it
currently pretends to represent the "transfer time").
Furthermore there is *zero* documentation describing this workaround,
not even in v6.
> Otherwise I will reserve myself a right to push a patch dropping CMD
> mode support until somebody comes with a proper way to handle CMD
> clock calculation.
Please do. That should force me or someone else to submit the right
calculations instead of repeatedly getting stuck in this loop of
complaints and incomprehensible patches with no fix in sight.
> It is off-topic for the sake of DSC 1.2 support. Yes, all CMD panel
> timings are a kind of a hack and should be improved. No, we can not do
> this as a part of this series. I think everybody agrees that with the
> current way of calculating CMD panel timings, this function does some
> kind of a trick.
I understand that you want to be pragrmatic about this situation, but it
seems wrong to build new math on top of fundamentally broken values. If
there's one thing I learned while contributing to mainline here, it is
that sometimes things are fundamentally broken and you cannot
ship/contribute a shiny new feature before first fixing the
fundamentals. What makes this different?
> > This function requires a documentation comment to explain that all.
> >
> >> + const struct drm_dsc_config *dsc)
> >> +{
> >> + int new_hdisplay = DIV_ROUND_UP(mode->hdisplay *
> >> drm_dsc_get_bpp_int(dsc),
> >
> > This sounds like a prime candidate for msm_dsc_get_bytes_per_line(), if
> > bits_per_component==8 is assumed. In fact, it then becomes identical
> > to the following line in dsi_host.c which you added previously:
> >
> > hdisplay = DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 3);
>
> This would imply a simple /3, but as far as I understand it is not
> correct here.
If you read Jessica's comment on that duplicate line in v6, that is
exactly what is happening:
* hdisplay will be divided by 3 here to account for the fact
* that DPU sends 3 bytes per pclk cycle to DSI.
That comment acknowledges that it is the same: but why isn't this? Why
is bits_per_component (number of bits per pixel component _in the source
picture_) suddenly introduced in this pclk calculation while it is not
used anywhere else?
> > If not, what is the difference between these two calculations? Maybe
> > they both need to be in a properly-named helper.
> >
> >> + dsc->bits_per_component * 3);
How is this different from dsi_get_bpp()?
> I hope to see a documentation patch to be posted, telling that this
> scales hdisplay and thus pclk by the factor of compressed_bpp /
> uncompressed_bpp.
>
> This is not how it is usually done, but I would accept a separate
> documentation patch going over the calculation here and in
> dsi_timing_setup (and maybe other unobvious cases, if there is anything
> left).
I'd
Re: [Freedreno] [PATCH v5 2/5] drm/msm/dsi: Adjust pclk rate for compression
On 2023-06-08 18:09:57, Abhinav Kumar wrote: > >> As discussed before we realized that this change is more-or-less a hack, > >> since downstream calculates pclk quite differently - at least for > >> command-mode panels. Do you still intend to land this patch this way, > >> or go the proper route by introducing the right math from the get-go? > >> Or is the math at least correct for video-mode panels? > > > > Sorry but can you please clarify what exactly is incorrect or different > > about this math when compared to downstream? And, if you think that this > > math is incorrect, what exactly has to be changed to make it the "right > > math"? > > > > Agree with Jessica, just calling the math a hack without explaining why > you think it is so is not justified especially when a great detail of > explanation was given on the bug. Sorry but its a bit harsh on the > developers. We discussed this in detail so I'm not quite sure why that suddenly needs to be reiterated again? - Marijn
Re: [PATCH] drm/i2c: Switch i2c drivers back to use .probe()
Uwe Kleine-König writes:
Hello Uwe,
> After commit b8a1a4cd5a98 ("i2c: Provide a temporary .probe_new()
> call-back type"), all drivers being converted to .probe_new() and then
> commit 03c835f498b5 ("i2c: Switch .probe() to not take an id parameter")
> convert back to (the new) .probe() to be able to eventually drop
> .probe_new() from struct i2c_driver.
>
> Signed-off-by: Uwe Kleine-König
> ---
Reviewed-by: Javier Martinez Canillas
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
[PATCH] drm/i2c: Switch i2c drivers back to use .probe()
After commit b8a1a4cd5a98 ("i2c: Provide a temporary .probe_new()
call-back type"), all drivers being converted to .probe_new() and then
commit 03c835f498b5 ("i2c: Switch .probe() to not take an id parameter")
convert back to (the new) .probe() to be able to eventually drop
.probe_new() from struct i2c_driver.
Signed-off-by: Uwe Kleine-König
---
drivers/gpu/drm/i2c/ch7006_drv.c | 2 +-
drivers/gpu/drm/i2c/sil164_drv.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i2c/ch7006_drv.c b/drivers/gpu/drm/i2c/ch7006_drv.c
index 521bdf656cca..131512a5f3bd 100644
--- a/drivers/gpu/drm/i2c/ch7006_drv.c
+++ b/drivers/gpu/drm/i2c/ch7006_drv.c
@@ -497,7 +497,7 @@ static const struct dev_pm_ops ch7006_pm_ops = {
static struct drm_i2c_encoder_driver ch7006_driver = {
.i2c_driver = {
- .probe_new = ch7006_probe,
+ .probe = ch7006_probe,
.remove = ch7006_remove,
.driver = {
diff --git a/drivers/gpu/drm/i2c/sil164_drv.c b/drivers/gpu/drm/i2c/sil164_drv.c
index f57f9a807542..ff23422727fc 100644
--- a/drivers/gpu/drm/i2c/sil164_drv.c
+++ b/drivers/gpu/drm/i2c/sil164_drv.c
@@ -420,7 +420,7 @@ MODULE_DEVICE_TABLE(i2c, sil164_ids);
static struct drm_i2c_encoder_driver sil164_driver = {
.i2c_driver = {
- .probe_new = sil164_probe,
+ .probe = sil164_probe,
.driver = {
.name = "sil164",
},
base-commit: 53ab6975c12d1ad86c599a8927e8c698b144d669
--
2.39.2
Re: [PATCH 30/30] fbdev: Make support for userspace interfaces configurable
Hi Thomas, On Mon, Jun 05, 2023 at 04:48:12PM +0200, Thomas Zimmermann wrote: > Add Kconfig option CONFIG_FB_DEVICE and make the virtual fbdev > device optional. If the new option has not been selected, fbdev > does not create a files in devfs or sysfs. s/ a// > > Most modern Linux systems run a DRM-based graphics stack that uses > the kernel's framebuffer console, but has otherwise deprecated fbdev > support. Yet fbdev userspace interfaces are still present. > > The option makes it possible to use the fbdev subsystem as console > implementation without support for userspace. This closes potential > entry points to manipulate kernel or I/O memory via framebuffers. It > also prevents the execution of driver code via ioctl or sysfs, both > of which might allow malicious software to exploit bugs in the fbdev > code. > > A small number of fbdev drivers require struct fbinfo.dev to be > initialized, usually for the support of sysfs interface. Make these > drivers depend on FB_DEVICE. They can later be fixed if necessary. Should that be a TODO in gpu/todo.rst? Otherwise the amount of people knowing about this is very close to 1. As an alternative add a TODO to each Kconfig file. > > Signed-off-by: Thomas Zimmermann > --- > drivers/staging/fbtft/Kconfig| 1 + > drivers/video/fbdev/Kconfig | 12 + > drivers/video/fbdev/core/Makefile| 7 +++--- > drivers/video/fbdev/core/fb_internal.h | 32 > drivers/video/fbdev/omap2/omapfb/Kconfig | 2 +- > include/linux/fb.h | 2 ++ > 6 files changed, 52 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig > index 4d29e8c1014e..5dda3c65a38e 100644 > --- a/drivers/staging/fbtft/Kconfig > +++ b/drivers/staging/fbtft/Kconfig > @@ -2,6 +2,7 @@ > menuconfig FB_TFT > tristate "Support for small TFT LCD display modules" > depends on FB && SPI > + depends on FB_DEVICE > depends on GPIOLIB || COMPILE_TEST > select FB_SYS_FILLRECT > select FB_SYS_COPYAREA > diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig > index 6df9bd09454a..48d9a14f889c 100644 > --- a/drivers/video/fbdev/Kconfig > +++ b/drivers/video/fbdev/Kconfig > @@ -57,6 +57,15 @@ config FIRMWARE_EDID > combination with certain motherboards and monitors are known to > suffer from this problem. > > +config FB_DEVICE > +bool "Provide legacy /dev/fb* device" > +depends on FB > +help > + Say Y here if you want the legacy /dev/fb* device file. It's > + only required if you have userspace programs that depend on > + fbdev for graphics output. This does not effect the framebuffer > + console. tabs to spaces to indent the above correct. > + > config FB_DDC > tristate > depends on FB > @@ -1545,6 +1554,7 @@ config FB_3DFX_I2C > config FB_VOODOO1 > tristate "3Dfx Voodoo Graphics (sst1) support" > depends on FB && PCI > + depends on FB_DEVICE > select FB_CFB_FILLRECT > select FB_CFB_COPYAREA > select FB_CFB_IMAGEBLIT > @@ -1862,6 +1872,7 @@ config FB_SH_MOBILE_LCDC > tristate "SuperH Mobile LCDC framebuffer support" > depends on FB && HAVE_CLK && HAS_IOMEM > depends on SUPERH || ARCH_RENESAS || COMPILE_TEST > + depends on FB_DEVICE > select FB_SYS_FILLRECT > select FB_SYS_COPYAREA > select FB_SYS_IMAGEBLIT > @@ -1930,6 +1941,7 @@ config FB_SMSCUFX > config FB_UDL > tristate "Displaylink USB Framebuffer support" > depends on FB && USB > + depends on FB_DEVICE > select FB_MODE_HELPERS > select FB_SYS_FILLRECT > select FB_SYS_COPYAREA > diff --git a/drivers/video/fbdev/core/Makefile > b/drivers/video/fbdev/core/Makefile > index 125d24f50c36..d5e8772620f8 100644 > --- a/drivers/video/fbdev/core/Makefile > +++ b/drivers/video/fbdev/core/Makefile > @@ -2,12 +2,13 @@ > obj-$(CONFIG_FB_NOTIFY) += fb_notify.o > obj-$(CONFIG_FB) += fb.o > fb-y := fb_backlight.o \ > - fb_devfs.o \ > fb_info.o \ > - fb_procfs.o \ > - fbmem.o fbmon.o fbcmap.o fbsysfs.o \ > + fbmem.o fbmon.o fbcmap.o \ > modedb.o fbcvt.o fb_cmdline.o > fb_io_fops.o > fb-$(CONFIG_FB_DEFERRED_IO) += fb_defio.o > +fb-$(CONFIG_FB_DEVICE)+= fb_devfs.o \ > + fb_procfs.o \ > + fbsysfs.o Maybe change this to one line to avoid '\'? > > ifeq ($(CONFIG_FRAMEBUFFER_CONSOLE),y) > fb-y += fbcon.o bitblit.o softcursor.o > diff --git a/drivers/video/fbdev/core/fb_internal.h >
[PATCH v2] drm/amdgpu: Add error reporting code for the display core
[why]
Because the drm/amdgpu drivers do not work with Navi 10 [RX 5700] series
GPUs on non-x86 platforms, this patch helps find out where the drivers fail.
After applying his patch, the following error message can be found:
[drm:dc_create [amdgpu]] *ERROR* dc_construct: failed to create resource pool
[drm:dc_create [amdgpu]] *ERROR* dc_create: dc construct failed
[drm] Display Core failed to initialize with v3.2.230!
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/amd/display/dc/core/dc.c | 29
1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 52564b93f7eb..d33b78aa3e58 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -951,7 +951,7 @@ static bool dc_construct(struct dc *dc,
goto fail;
}
-dc_ctx = dc->ctx;
+ dc_ctx = dc->ctx;
/* Resource should construct all asic specific resources.
* This should be the only place where we need to parse the asic id
@@ -990,16 +990,21 @@ static bool dc_construct(struct dc *dc,
}
dc->res_pool = dc_create_resource_pool(dc, init_params,
dc_ctx->dce_version);
- if (!dc->res_pool)
+ if (!dc->res_pool) {
+ dm_error("%s: failed to create resource pool\n", __func__);
goto fail;
+ }
/* set i2c speed if not done by the respective dcnxxx__resource.c */
if (dc->caps.i2c_speed_in_khz_hdcp == 0)
dc->caps.i2c_speed_in_khz_hdcp = dc->caps.i2c_speed_in_khz;
dc->clk_mgr = dc_clk_mgr_create(dc->ctx, dc->res_pool->pp_smu,
dc->res_pool->dccg);
- if (!dc->clk_mgr)
+ if (!dc->clk_mgr) {
+ dm_error("%s: failed to create clk manager\n", __func__);
goto fail;
+ }
+
#ifdef CONFIG_DRM_AMD_DC_FP
dc->clk_mgr->force_smu_not_present = init_params->force_smu_not_present;
@@ -1022,14 +1027,18 @@ static bool dc_construct(struct dc *dc,
goto fail;
}
- if (!create_links(dc, init_params->num_virtual_links))
+ if (!create_links(dc, init_params->num_virtual_links)) {
+ dm_error("%s: failed to create links\n", __func__);
goto fail;
+ }
/* Create additional DIG link encoder objects if fewer than the platform
* supports were created during link construction.
*/
- if (!create_link_encoders(dc))
+ if (!create_link_encoders(dc)) {
+ dm_error("%s: failed to create link encoders\n", __func__);
goto fail;
+ }
dc_resource_state_construct(dc, dc->current_state);
@@ -1314,11 +1323,15 @@ struct dc *dc_create(const struct dc_init_data
*init_params)
return NULL;
if (init_params->dce_environment == DCE_ENV_VIRTUAL_HW) {
- if (!dc_construct_ctx(dc, init_params))
+ if (!dc_construct_ctx(dc, init_params)) {
+ DC_LOG_ERROR("%s: dc construct failed\n", __func__);
goto destruct_dc;
+ }
} else {
- if (!dc_construct(dc, init_params))
+ if (!dc_construct(dc, init_params)) {
+ DC_LOG_ERROR("%s: dc construct failed\n", __func__);
goto destruct_dc;
+ }
full_pipe_count = dc->res_pool->pipe_count;
if (dc->res_pool->underlay_pipe_index != NO_UNDERLAY_PIPE)
@@ -1349,8 +1362,6 @@ struct dc *dc_create(const struct dc_init_data
*init_params)
DC_LOG_DC("Display Core initialized\n");
-
-
return dc;
destruct_dc:
--
2.25.1
RE: [PATCH v17 1/1] drm/i915: Allow user to set cache at BO creation
It is impossible to pass the PAT setting when sharing the fd. 1. All api , include vaapi, vulkan, gl... need such extension , the impact is huge. Even these API level accept , some middle ware , such as ffmpeg... also will reject it. 2. PAT is intel specific implementation , and MTL , LNL has different value and meanings, you could not expect application developer understand the details, it should be transparent to application developer. Thanks Carl From: Yang, Fei Sent: Friday, June 9, 2023 11:13 PM To: Andi Shyti ; Zhang, Carl Cc: Joonas Lahtinen ; Tvrtko Ursulin ; Chris Wilson ; Roper, Matthew D ; Justen, Jordan L ; Gu, Lihao ; Intel GFX ; DRI Devel Subject: Re: [PATCH v17 1/1] drm/i915: Allow user to set cache at BO creation > Hi Carl, > besides this, ask a dumb question. How we retrieve the pat_index from a shared resource though dma_buf fd? maybe we need to know whether it could be CPU cached if we want map it. Of course, looks there are no real usage to access it though CPU. Just use it directly without any pat related options ? >>> >>> I am not understanding. Do you want to ask the PAT table to the driver? Are >>> you referring to the CPU PAT index? >>> >>> In any case, if I understood correctly, you don't necessarily always need to >>> set the PAT options and the cache options will fall into the default values. >>> >>> Please let me know if I haven't answered the question. >>> >> >> If mesa create a resource , then use DRM_IOCTL_PRIME_HANDLE_TO_FD convert it >> to a dma fd. >> Then share it to media, media use DRM_IOCTL_PRIME_FD_TO_HANDLE convert it to >> a gem bo. >> But media does not know the PAT index , because mesa create it and set it. >> So, if media want to call DRM_IOCTL_I915_GEM_MMAP_OFFSET, media does not >> know whether it could be WB. > > That's a good point. To be honest I am not really sure how this > is handled. > > Fei, Jordan? Do you have suggestion here? Is it possible to pass the PAT setting when sharing the fd? Or perhaps we should have kept the get_caching ioctl functional? Joonas, could you chime in here? > Andi
Re: [PATCH] accel/habanalabs: add more debugfs stub helpers
On Fri, Jun 9, 2023 at 4:37 PM Tomer Tayar wrote:
>
> On 09/06/2023 15:06, Arnd Bergmann wrote:
> > From: Arnd Bergmann
> >
> > Two functions got added with normal prototypes for debugfs, but not
> > alternative when building without it:
> >
> > drivers/accel/habanalabs/common/device.c: In function 'hl_device_init':
> > drivers/accel/habanalabs/common/device.c:2177:14: error: implicit
> > declaration of function 'hl_debugfs_device_init'; did you mean
> > 'hl_debugfs_init'? [-Werror=implicit-function-declaration]
> > drivers/accel/habanalabs/common/device.c:2305:9: error: implicit
> > declaration of function 'hl_debugfs_device_fini'; did you mean
> > 'hl_debugfs_remove_file'? [-Werror=implicit-function-declaration]
> >
> > Add stubs for these as well.
> >
> > Fixes: 553311fc7b76e ("accel/habanalabs: expose debugfs files later")
> > Signed-off-by: Arnd Bergmann
>
> Thanks,
> Reviewed-by: Tomer Tayar
Thanks,
Applied to -fixes.
Oded
>
> > ---
> > drivers/accel/habanalabs/common/habanalabs.h | 9 +
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/accel/habanalabs/common/habanalabs.h
> > b/drivers/accel/habanalabs/common/habanalabs.h
> > index d92ba2e30e310..2f027d5a82064 100644
> > --- a/drivers/accel/habanalabs/common/habanalabs.h
> > +++ b/drivers/accel/habanalabs/common/habanalabs.h
> > @@ -3980,6 +3980,15 @@ static inline void hl_debugfs_fini(void)
> > {
> > }
> >
> > +static inline int hl_debugfs_device_init(struct hl_device *hdev)
> > +{
> > + return 0;
> > +}
> > +
> > +static inline void hl_debugfs_device_fini(struct hl_device *hdev)
> > +{
> > +}
> > +
> > static inline void hl_debugfs_add_device(struct hl_device *hdev)
> > {
> > }
>
>
Re: [RESEND, 12/15] drm/nouveau/dispnv04/crtc: Demote kerneldoc abuses
Reviewed-by: Sui Jingfeng On 2023/6/9 16:17, Lee Jones wrote: Fixes the following W=1 kernel build warning(s): drivers/gpu/drm/nouveau/dispnv04/crtc.c:453: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst drivers/gpu/drm/nouveau/dispnv04/crtc.c:629: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst Cc: Ben Skeggs Cc: Karol Herbst Cc: Lyude Paul Cc: David Airlie Cc: Daniel Vetter Cc: dri-devel@lists.freedesktop.org Cc: nouv...@lists.freedesktop.org Signed-off-by: Lee Jones Reviewed-by: Karol Herbst --- drivers/gpu/drm/nouveau/dispnv04/crtc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c b/drivers/gpu/drm/nouveau/dispnv04/crtc.c index a6f2e681bde98..7794902df17d5 100644 --- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c +++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c @@ -449,7 +449,7 @@ nv_crtc_mode_set_vga(struct drm_crtc *crtc, struct drm_display_mode *mode) regp->Attribute[NV_CIO_AR_CSEL_INDEX] = 0x00; } -/** +/* * Sets up registers for the given mode/adjusted_mode pair. * * The clocks, CRTCs and outputs attached to this CRTC must be off. @@ -625,7 +625,7 @@ nv_crtc_swap_fbs(struct drm_crtc *crtc, struct drm_framebuffer *old_fb) return ret; } -/** +/* * Sets up registers for the given mode/adjusted_mode pair. * * The clocks, CRTCs and outputs attached to this CRTC must be off. -- Jingfeng
Re: [PATCH] drm: etnaviv: Replace of_platform.h with explicit includes
Reviewed-by: Sui Jingfeng On 2023/6/10 04:17, Rob Herring wrote: On Mon, Apr 10, 2023 at 5:26 PM Rob Herring wrote: Etnaviv doesn't use anything from of_platform.h, but depends on of.h, of_device.h, and platform_device.h which are all implicitly included, but that is going to be removed soon. Signed-off-by: Rob Herring --- drivers/gpu/drm/etnaviv/etnaviv_drv.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) Ping! of_device.h already has 'linux/of.h' and 'linux/platform_device.h' included, Would it be sufficient by simply including linux/of_device.h ? I'm fine with the above question explained. ``` #include #include /* temporary until merge */ #include ``` diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c index 44ca803237a5..c68e83ed5a23 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c @@ -6,7 +6,9 @@ #include #include #include -#include +#include +#include +#include #include #include -- 2.39.2 -- Jingfeng
Re: [PATCH] drm/edid: Add quirk for OSVR HDK 2.0
On 6/10/23 00:22, Jani Nikula wrote:
On Fri, 09 Jun 2023, Ralph Campbell wrote:
On 6/9/23 02:03, Jani Nikula wrote:
On Thu, 08 Jun 2023, Ralph Campbell wrote:
The OSVR virtual reality headset HDK 2.0 uses a different EDID
vendor and device identifier than the HDK 1.1 - 1.4 headsets.
Add the HDK 2.0 vendor and device identifier to the quirks table so
that window managers do not try to display the desktop screen on the
headset display.
At some point in time we requested bugs to be filed about quirks, with
EDIDs attached, so we could look at them later, and maybe remove the
quirks.
The headset non-desktop thing started off as a quirk, but since then
we've added both Microsoft VSDB and DisplayID primary use as ways to
indicate this without quirks.
BR,
Jani.
If you want me to file a bug, I can do that and I have the EDID too.
Where would I file it?
I suppose at https://gitlab.freedesktop.org/drm/misc/-/issues
We should then reference the issue in the commit message (no need to
resend, this can be added while applying).
I did see the DisplayID 2.0 code. This headset is no longer being
manufactured so updating the EDID is not practical.
I'm not saying the EDID should be updated, just that we might drop the
quirk if we find another generic way to identify non-desktops that
covers the EDID in question.
If the device isn't covered by the existing mechanisms, I'm not opposed
to merging as-is, with the issue reference added.
Thanks,
Jani.
Thanks, I created issue #30
"OSVR virtual reality headset HDK 2.0 not recognized as a non-desktop display"
Signed-off-by: Ralph Campbell
Tested-by: Ralph Campbell
---
drivers/gpu/drm/drm_edid.c | 1 +
1 file changed, 1 insertion(+)
I don't know how many of these VR headsets are still around but I have a
working one and I saw and entry for HDK 1.x so I thought it would be good
to add HDK 2.0.
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 0454da505687..3b8cc1fe05e8 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -230,6 +230,7 @@ static const struct edid_quirk {
/* OSVR HDK and HDK2 VR Headsets */
EDID_QUIRK('S', 'V', 'R', 0x1019, EDID_QUIRK_NON_DESKTOP),
+ EDID_QUIRK('A', 'O', 'U', 0x, EDID_QUIRK_NON_DESKTOP),
};
/*
