Re: [PATCH 02/36] drm/drm_property: make replace_property_blob_from_id a DRM helper
On Tue, May 23, 2023 at 09:14:46PM -0100, Melissa Wen wrote:
> Place it in drm_property where drm_property_replace_blob and
> drm_property_lookup_blob live. Then we can use the DRM helper for
> driver-specific KMS properties too.
>
> Signed-off-by: Melissa Wen
I know that I've got Cc-ed because of a comment, but I did have a look at the
whole
patch. If it is useful, then you can add
Reviewed-by: Liviu Dudau
Best regards,
Liviu
> ---
> drivers/gpu/drm/arm/malidp_crtc.c | 2 +-
> drivers/gpu/drm/drm_atomic_uapi.c | 43 ---
> drivers/gpu/drm/drm_property.c| 49 +++
> include/drm/drm_property.h| 6
> 4 files changed, 61 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/malidp_crtc.c
> b/drivers/gpu/drm/arm/malidp_crtc.c
> index dc01c43f6193..d72c22dcf685 100644
> --- a/drivers/gpu/drm/arm/malidp_crtc.c
> +++ b/drivers/gpu/drm/arm/malidp_crtc.c
> @@ -221,7 +221,7 @@ static int malidp_crtc_atomic_check_ctm(struct drm_crtc
> *crtc,
>
> /*
>* The size of the ctm is checked in
> - * drm_atomic_replace_property_blob_from_id.
> + * drm_property_replace_blob_from_id.
>*/
> ctm = (struct drm_color_ctm *)state->ctm->data;
> for (i = 0; i < ARRAY_SIZE(ctm->matrix); ++i) {
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index c06d0639d552..b76d50ae244c 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -362,39 +362,6 @@ static s32 __user *get_out_fence_for_connector(struct
> drm_atomic_state *state,
> return fence_ptr;
> }
>
> -static int
> -drm_atomic_replace_property_blob_from_id(struct drm_device *dev,
> - struct drm_property_blob **blob,
> - uint64_t blob_id,
> - ssize_t expected_size,
> - ssize_t expected_elem_size,
> - bool *replaced)
> -{
> - struct drm_property_blob *new_blob = NULL;
> -
> - if (blob_id != 0) {
> - new_blob = drm_property_lookup_blob(dev, blob_id);
> - if (new_blob == NULL)
> - return -EINVAL;
> -
> - if (expected_size > 0 &&
> - new_blob->length != expected_size) {
> - drm_property_blob_put(new_blob);
> - return -EINVAL;
> - }
> - if (expected_elem_size > 0 &&
> - new_blob->length % expected_elem_size != 0) {
> - drm_property_blob_put(new_blob);
> - return -EINVAL;
> - }
> - }
> -
> - *replaced |= drm_property_replace_blob(blob, new_blob);
> - drm_property_blob_put(new_blob);
> -
> - return 0;
> -}
> -
> static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> struct drm_crtc_state *state, struct drm_property *property,
> uint64_t val)
> @@ -415,7 +382,7 @@ static int drm_atomic_crtc_set_property(struct drm_crtc
> *crtc,
> } else if (property == config->prop_vrr_enabled) {
> state->vrr_enabled = val;
> } else if (property == config->degamma_lut_property) {
> - ret = drm_atomic_replace_property_blob_from_id(dev,
> + ret = drm_property_replace_blob_from_id(dev,
> >degamma_lut,
> val,
> -1, sizeof(struct drm_color_lut),
> @@ -423,7 +390,7 @@ static int drm_atomic_crtc_set_property(struct drm_crtc
> *crtc,
> state->color_mgmt_changed |= replaced;
> return ret;
> } else if (property == config->ctm_property) {
> - ret = drm_atomic_replace_property_blob_from_id(dev,
> + ret = drm_property_replace_blob_from_id(dev,
> >ctm,
> val,
> sizeof(struct drm_color_ctm), -1,
> @@ -431,7 +398,7 @@ static int drm_atomic_crtc_set_property(struct drm_crtc
> *crtc,
> state->color_mgmt_changed |= replaced;
> return ret;
> } else if (property == config->gamma_lut_property) {
> - ret = drm_atomic_replace_property_blob_from_id(dev,
> + ret = drm_property_replace_blob_from_id(dev,
> >gamma_lut,
>
Re: [PATCH v2 02/10] drm: Include where needed
On Wed, Jan 11, 2023 at 02:01:58PM +0100, Thomas Zimmermann wrote: > Include in source files that need it. Some of DRM's > source code gets OF header via drm_crtc_helper.h and , > which can leed to unnecessary recompilation. > > In drm_modes.c, add a comment on the reason for still including > . The header file is required to get KHZ2PICOS(). The > macro is part of the UAPI headers, so it cannot be moved to a less > prominent location. > > v2: > * include in komeda_drv.c (kernel test robot) > > Signed-off-by: Thomas Zimmermann > --- > drivers/gpu/drm/arm/display/komeda/komeda_drv.c | 1 + > drivers/gpu/drm/drm_modes.c | 5 +++-- > drivers/gpu/drm/panel/panel-ronbo-rb070d30.c| 1 + > 3 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c > b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c > index 3f4e719eebd8..28f76e07dd95 100644 > --- a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c > @@ -6,6 +6,7 @@ > */ > #include > #include > +#include > #include > #include > #include For komeda: Acked-by: Liviu Dudau Best regards, Liviu > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > index be030f4a5311..40d482a01178 100644 > --- a/drivers/gpu/drm/drm_modes.c > +++ b/drivers/gpu/drm/drm_modes.c > @@ -31,10 +31,11 @@ > */ > > #include > +#include > +#include /* for KHZ2PICOS() */ > #include > #include > -#include > -#include > +#include > > #include > #include > diff --git a/drivers/gpu/drm/panel/panel-ronbo-rb070d30.c > b/drivers/gpu/drm/panel/panel-ronbo-rb070d30.c > index a8a98c91b13c..866d1bf5530e 100644 > --- a/drivers/gpu/drm/panel/panel-ronbo-rb070d30.c > +++ b/drivers/gpu/drm/panel/panel-ronbo-rb070d30.c > @@ -15,6 +15,7 @@ > #include > #include > #include > +#include > > #include > #include > -- > 2.39.0 > -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ツ)_/¯
Re: [PATCH v2 02/14] drm/arm/hdlcd: Convert to Linux IRQ interfaces
On Tue, Aug 03, 2021 at 11:06:52AM +0200, Thomas Zimmermann wrote:
> Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
> IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
> don't benefit from using it.
>
> DRM IRQ callbacks are now being called directly or inlined.
>
> Calls to platform_get_irq() can fail with a negative errno code.
> Abort initialization in this case. The DRM IRQ midlayer does not
> handle this case correctly.
>
> v2:
> * name struct drm_device variables 'drm' (Sam)
>
> Signed-off-by: Thomas Zimmermann
> Acked-by: Sam Ravnborg
Sorry for the delayed response due to holidays.
Acked-by: Liviu Dudau
Best regards,
Liviu
> ---
> drivers/gpu/drm/arm/hdlcd_drv.c | 174 ++--
> drivers/gpu/drm/arm/hdlcd_drv.h | 1 +
> 2 files changed, 97 insertions(+), 78 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
> index 81ae92390736..479c2422a2e0 100644
> --- a/drivers/gpu/drm/arm/hdlcd_drv.c
> +++ b/drivers/gpu/drm/arm/hdlcd_drv.c
> @@ -29,7 +29,6 @@
> #include
> #include
> #include
> -#include
> #include
> #include
> #include
> @@ -38,6 +37,94 @@
> #include "hdlcd_drv.h"
> #include "hdlcd_regs.h"
>
> +static irqreturn_t hdlcd_irq(int irq, void *arg)
> +{
> + struct drm_device *drm = arg;
> + struct hdlcd_drm_private *hdlcd = drm->dev_private;
> + unsigned long irq_status;
> +
> + irq_status = hdlcd_read(hdlcd, HDLCD_REG_INT_STATUS);
> +
> +#ifdef CONFIG_DEBUG_FS
> + if (irq_status & HDLCD_INTERRUPT_UNDERRUN)
> + atomic_inc(>buffer_underrun_count);
> +
> + if (irq_status & HDLCD_INTERRUPT_DMA_END)
> + atomic_inc(>dma_end_count);
> +
> + if (irq_status & HDLCD_INTERRUPT_BUS_ERROR)
> + atomic_inc(>bus_error_count);
> +
> + if (irq_status & HDLCD_INTERRUPT_VSYNC)
> + atomic_inc(>vsync_count);
> +
> +#endif
> + if (irq_status & HDLCD_INTERRUPT_VSYNC)
> + drm_crtc_handle_vblank(>crtc);
> +
> + /* acknowledge interrupt(s) */
> + hdlcd_write(hdlcd, HDLCD_REG_INT_CLEAR, irq_status);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void hdlcd_irq_preinstall(struct drm_device *drm)
> +{
> + struct hdlcd_drm_private *hdlcd = drm->dev_private;
> + /* Ensure interrupts are disabled */
> + hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, 0);
> + hdlcd_write(hdlcd, HDLCD_REG_INT_CLEAR, ~0);
> +}
> +
> +static void hdlcd_irq_postinstall(struct drm_device *drm)
> +{
> +#ifdef CONFIG_DEBUG_FS
> + struct hdlcd_drm_private *hdlcd = drm->dev_private;
> + unsigned long irq_mask = hdlcd_read(hdlcd, HDLCD_REG_INT_MASK);
> +
> + /* enable debug interrupts */
> + irq_mask |= HDLCD_DEBUG_INT_MASK;
> +
> + hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, irq_mask);
> +#endif
> +}
> +
> +static int hdlcd_irq_install(struct drm_device *drm, int irq)
> +{
> + int ret;
> +
> + if (irq == IRQ_NOTCONNECTED)
> + return -ENOTCONN;
> +
> + hdlcd_irq_preinstall(drm);
> +
> + ret = request_irq(irq, hdlcd_irq, 0, drm->driver->name, drm);
> + if (ret)
> + return ret;
> +
> + hdlcd_irq_postinstall(drm);
> +
> + return 0;
> +}
> +
> +static void hdlcd_irq_uninstall(struct drm_device *drm)
> +{
> + struct hdlcd_drm_private *hdlcd = drm->dev_private;
> + /* disable all the interrupts that we might have enabled */
> + unsigned long irq_mask = hdlcd_read(hdlcd, HDLCD_REG_INT_MASK);
> +
> +#ifdef CONFIG_DEBUG_FS
> + /* disable debug interrupts */
> + irq_mask &= ~HDLCD_DEBUG_INT_MASK;
> +#endif
> +
> + /* disable vsync interrupts */
> + irq_mask &= ~HDLCD_INTERRUPT_VSYNC;
> + hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, irq_mask);
> +
> + free_irq(hdlcd->irq, drm);
> +}
> +
> static int hdlcd_load(struct drm_device *drm, unsigned long flags)
> {
> struct hdlcd_drm_private *hdlcd = drm->dev_private;
> @@ -90,7 +177,12 @@ static int hdlcd_load(struct drm_device *drm, unsigned
> long flags)
> goto setup_fail;
> }
>
> - ret = drm_irq_install(drm, platform_get_irq(pdev, 0));
> + ret = platform_get_irq(pdev, 0);
> + if (ret < 0)
> + goto irq_fail;
> + hdlcd->irq = ret;
> +
> + ret = hdlcd_irq_install(drm, hdlcd->irq);
> if (ret < 0) {
> DRM_ERROR("failed to install IRQ handler\n");
>
[PATCH] drm/amd/display: Fix 10bit 4K display on CIK GPUs
Commit 72a7cf0aec0c ("drm/amd/display: Keep linebuffer pixel depth at
30bpp for DCE-11.0.") doesn't seems to have fixed 10bit 4K rendering over
DisplayPort for CIK GPUs. On my machine with a HAWAII GPU I get a broken
image that looks like it has an effective resolution of 1920x1080 but
scaled up in an irregular way. Reverting the commit or applying this
patch fixes the problem on v5.14-rc1.
Fixes: 72a7cf0aec0c ("drm/amd/display: Keep linebuffer pixel depth at 30bpp for
DCE-11.0.")
Signed-off-by: Liviu Dudau
---
drivers/gpu/drm/amd/display/dc/core/dc_resource.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
index a6a67244a322e..1596f6b7fed7c 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
@@ -1062,7 +1062,7 @@ bool resource_build_scaling_params(struct pipe_ctx
*pipe_ctx)
* so use only 30 bpp on DCE_VERSION_11_0. Testing with DCE 11.2 and 8.3
* did not show such problems, so this seems to be the exception.
*/
- if (plane_state->ctx->dce_version != DCE_VERSION_11_0)
+ if (plane_state->ctx->dce_version > DCE_VERSION_11_0)
pipe_ctx->plane_res.scl_data.lb_params.depth =
LB_PIXEL_DEPTH_36BPP;
else
pipe_ctx->plane_res.scl_data.lb_params.depth =
LB_PIXEL_DEPTH_30BPP;
--
2.32.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v3 04/27] drm: Don't test for IRQ support in VBLANK ioctls
On Thu, Jun 24, 2021 at 11:07:57AM +0200, Thomas Zimmermann wrote:
> Hi
>
> Am 24.06.21 um 10:51 schrieb Jani Nikula:
> > On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
> > > Hi
> > >
> > > Am 24.06.21 um 10:06 schrieb Jani Nikula:
> > > > On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
> > > > > diff --git a/drivers/gpu/drm/drm_vblank.c
> > > > > b/drivers/gpu/drm/drm_vblank.c
> > > > > index 3417e1ac7918..10fe16bafcb6 100644
> > > > > --- a/drivers/gpu/drm/drm_vblank.c
> > > > > +++ b/drivers/gpu/drm/drm_vblank.c
> > > > > @@ -1748,8 +1748,16 @@ int drm_wait_vblank_ioctl(struct drm_device
> > > > > *dev, void *data,
> > > > > unsigned int pipe_index;
> > > > > unsigned int flags, pipe, high_pipe;
> > > > > - if (!dev->irq_enabled)
> > > > > - return -EOPNOTSUPP;
> > > > > +#if defined(CONFIG_DRM_LEGACY)
> > > > > + if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY))) {
> > > > > + if (!dev->irq_enabled)
> > > > > + return -EOPNOTSUPP;
> > > > > + } else /* if DRIVER_MODESET */
> > > > > +#endif
> > > > > + {
> > > > > + if (!drm_dev_has_vblank(dev))
> > > > > + return -EOPNOTSUPP;
> > > > > + }
> > > >
> > > > Sheesh I hate this kind of inline #ifdefs.
> > > >
> > > > Two alternate suggestions that I believe should be as just efficient:
> > >
> > > Or how about:
> > >
> > > static bool drm_wait_vblank_supported(struct drm_device *dev)
> > >
> > > {
> > >
> > > if defined(CONFIG_DRM_LEGACY)
> > > if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
> > >
> > > return dev->irq_enabled;
> > >
> > > #endif
> > > return drm_dev_has_vblank(dev);
> > >
> > > }
> > >
> > >
> > > ?
> > >
> > > It's inline, but still readable.
> >
> > It's definitely better than the original, but it's unclear to me why
> > you'd prefer this over option 2) below. I guess the only reason I can
> > think of is emphasizing the conditional compilation. However,
> > IS_ENABLED() is widely used in this manner specifically to avoid inline
> > #if, and the compiler optimizes it away.
>
> It's simply more readable to me as the condition is simpler. But option 2 is
> also ok.
Either option looks good to me.
Reviewed-by: Liviu Dudau
Thanks for doing that!
Liviu
>
> Best regards
> Thomas
>
> >
> > BR,
> > Jani.
> >
> >
> > >
> > > Best regards
> > > Thomas
> > >
> > > >
> > > > 1) The more verbose:
> > > >
> > > > #if defined(CONFIG_DRM_LEGACY)
> > > > static bool drm_wait_vblank_supported(struct drm_device *dev)
> > > > {
> > > > if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
> > > > return dev->irq_enabled;
> > > > else
> > > > return drm_dev_has_vblank(dev);
> > > > }
> > > > #else
> > > > static bool drm_wait_vblank_supported(struct drm_device *dev)
> > > > {
> > > > return drm_dev_has_vblank(dev);
> > > > }
> > > > #endif
> > > >
> > > > 2) The more compact:
> > > >
> > > > static bool drm_wait_vblank_supported(struct drm_device *dev)
> > > > {
> > > > if (IS_ENABLED(CONFIG_DRM_LEGACY) &&
> > > > unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
> > > > return dev->irq_enabled;
> > > > else
> > > > return drm_dev_has_vblank(dev);
> > > > }
> > > >
> > > > Then, in drm_wait_vblank_ioctl().
> > > >
> > > > if (!drm_wait_vblank_supported(dev))
> > > > return -EOPNOTSUPP;
> > > >
> > > > The compiler should do the right thing without any explicit inline
> > > > keywords etc.
> > > >
> > > > BR,
> > > > Jani.
> > > >
> > > > > if (vblwait->request.type & _DRM_VBLANK_SIGNAL)
> &g
Re: [PATCH v2 04/22] drm: Don't test for IRQ support in VBLANK ioctls
Hi Thomas,
On Wed, Jun 23, 2021 at 08:43:07AM +0200, Thomas Zimmermann wrote:
> Hi Liviu
>
> Am 22.06.21 um 17:25 schrieb Liviu Dudau:
> > Hello,
> >
> > On Tue, Jun 22, 2021 at 04:09:44PM +0200, Thomas Zimmermann wrote:
> > > For KMS drivers, replace the IRQ check in VBLANK ioctls with a check for
> > > vblank support. IRQs might be enabled wthout vblanking being supported.
> > >
> > > This change also removes the DRM framework's only dependency on IRQ state
> > > for non-legacy drivers. For legacy drivers with userspace modesetting,
> > > the original test remains in drm_wait_vblank_ioctl().
> > >
> > > v2:
> > > * keep the old test for legacy drivers in
> > > drm_wait_vblank_ioctl() (Daniel)
> > >
> > > Signed-off-by: Thomas Zimmermann
> > > ---
> > > drivers/gpu/drm/drm_irq.c| 10 +++---
> > > drivers/gpu/drm/drm_vblank.c | 13 +
> > > 2 files changed, 12 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> > > index c3bd664ea733..1d7785721323 100644
> > > --- a/drivers/gpu/drm/drm_irq.c
> > > +++ b/drivers/gpu/drm/drm_irq.c
> > > @@ -74,10 +74,8 @@
> > >* only supports devices with a single interrupt on the main device
> > > stored in
> > >* _device.dev and set as the device paramter in drm_dev_alloc().
> > >*
> > > - * These IRQ helpers are strictly optional. Drivers which roll their own
> > > only
> > > - * need to set _device.irq_enabled to signal the DRM core that vblank
> > > - * interrupts are working. Since these helpers don't automatically clean
> > > up the
> > > - * requested interrupt like e.g. devm_request_irq() they're not really
> > > + * These IRQ helpers are strictly optional. Since these helpers don't
> > > automatically
> > > + * clean up the requested interrupt like e.g. devm_request_irq() they're
> > > not really
> > >* recommended.
> > >*/
> > > @@ -91,9 +89,7 @@
> > >* and after the installation.
> > >*
> > >* This is the simplified helper interface provided for drivers with no
> > > special
> > > - * needs. Drivers which need to install interrupt handlers for multiple
> > > - * interrupts must instead set _device.irq_enabled to signal the DRM
> > > core
> > > - * that vblank interrupts are available.
> > > + * needs.
> > >*
> > >* @irq must match the interrupt number that would be passed to
> > > request_irq(),
> > >* if called directly instead of using this helper function.
> > > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> > > index 3417e1ac7918..a98a4aad5037 100644
> > > --- a/drivers/gpu/drm/drm_vblank.c
> > > +++ b/drivers/gpu/drm/drm_vblank.c
> > > @@ -1748,8 +1748,13 @@ int drm_wait_vblank_ioctl(struct drm_device *dev,
> > > void *data,
> > > unsigned int pipe_index;
> > > unsigned int flags, pipe, high_pipe;
> > > - if (!dev->irq_enabled)
> > > - return -EOPNOTSUPP;
> > > + if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> > > + if (!drm_dev_has_vblank(dev))
> > > + return -EOPNOTSUPP;
> > > + } else {
> > > + if (!dev->irq_enabled)
> > > + return -EOPNOTSUPP;
> > > + }
> >
> > For a system call that is used quite a lot by userspace we have increased
> > the code size
> > in a noticeable way. Can we not cache it privately?
>
> I'm not quite sure that I understand your concern. The additionally called
> functions are trivial one-liners; probably inlined anyway.
They are inlined. However we replace the pointer dereference (which can be
calculated
at compile time as offset from a base pointer) with the code in
drm_core_check_all_features() that does 3 pointer dereferences, masking and
logical
AND before checking for matching value.
>
> However, irq_enabled is only relevant for legacy drivers and will eventually
> disappear behind CONFIG_DRM_LEGACY. We can rewrite the test like this:
I get the point that irq_enabled is legacy. However the IOCTL call is not and
usually
is used in time critical code to wait for vblank before starting the old
buffers for
a new frame. At 60Hz that's probably less of a concern, but at 120Hz refresh
rate and
reduced vblank time your time slice allocation for new work matters.
Best regards,
Re: [PATCH v2 06/22] drm/malidp: Don't set struct drm_device.irq_enabled
On Tue, Jun 22, 2021 at 04:09:46PM +0200, Thomas Zimmermann wrote:
> The field drm_device.irq_enabled is only used by legacy drivers
> with userspace modesetting. Don't set it in malidp.
>
> Signed-off-by: Thomas Zimmermann
Acked-by: Liviu Dudau
Best regards,
Liviu
> ---
> drivers/gpu/drm/arm/malidp_drv.c | 4
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/malidp_drv.c
> b/drivers/gpu/drm/arm/malidp_drv.c
> index de59f3302516..78d15b04b105 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -847,8 +847,6 @@ static int malidp_bind(struct device *dev)
> if (ret < 0)
> goto irq_init_fail;
>
> - drm->irq_enabled = true;
> -
> ret = drm_vblank_init(drm, drm->mode_config.num_crtc);
> if (ret < 0) {
> DRM_ERROR("failed to initialise vblank\n");
> @@ -874,7 +872,6 @@ static int malidp_bind(struct device *dev)
> vblank_fail:
> malidp_se_irq_fini(hwdev);
> malidp_de_irq_fini(hwdev);
> - drm->irq_enabled = false;
> irq_init_fail:
> drm_atomic_helper_shutdown(drm);
> component_unbind_all(dev, drm);
> @@ -909,7 +906,6 @@ static void malidp_unbind(struct device *dev)
> drm_atomic_helper_shutdown(drm);
> malidp_se_irq_fini(hwdev);
> malidp_de_irq_fini(hwdev);
> - drm->irq_enabled = false;
> component_unbind_all(dev, drm);
> of_node_put(malidp->crtc.port);
> malidp->crtc.port = NULL;
> --
> 2.32.0
>
--
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---
¯\_(ツ)_/¯
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2 05/22] drm/komeda: Don't set struct drm_device.irq_enabled
On Tue, Jun 22, 2021 at 04:09:45PM +0200, Thomas Zimmermann wrote: > The field drm_device.irq_enabled is only used by legacy drivers > with userspace modesetting. Don't set it in komeda. > > Signed-off-by: Thomas Zimmermann Acked-by: Liviu Dudau Best regards, Liviu > --- > drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 4 > 1 file changed, 4 deletions(-) > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c > b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c > index ff45f23f3d56..52a6db5707a3 100644 > --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c > @@ -301,8 +301,6 @@ struct komeda_kms_dev *komeda_kms_attach(struct > komeda_dev *mdev) > if (err) > goto free_component_binding; > > - drm->irq_enabled = true; > - > drm_kms_helper_poll_init(drm); > > err = drm_dev_register(drm, 0); > @@ -313,7 +311,6 @@ struct komeda_kms_dev *komeda_kms_attach(struct > komeda_dev *mdev) > > free_interrupts: > drm_kms_helper_poll_fini(drm); > - drm->irq_enabled = false; > free_component_binding: > component_unbind_all(mdev->dev, drm); > cleanup_mode_config: > @@ -331,7 +328,6 @@ void komeda_kms_detach(struct komeda_kms_dev *kms) > drm_dev_unregister(drm); > drm_kms_helper_poll_fini(drm); > drm_atomic_helper_shutdown(drm); > - drm->irq_enabled = false; > component_unbind_all(mdev->dev, drm); > drm_mode_config_cleanup(drm); > komeda_kms_cleanup_private_objs(kms); > -- > 2.32.0 > -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ツ)_/¯ ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2 04/22] drm: Don't test for IRQ support in VBLANK ioctls
Hello,
On Tue, Jun 22, 2021 at 04:09:44PM +0200, Thomas Zimmermann wrote:
> For KMS drivers, replace the IRQ check in VBLANK ioctls with a check for
> vblank support. IRQs might be enabled wthout vblanking being supported.
>
> This change also removes the DRM framework's only dependency on IRQ state
> for non-legacy drivers. For legacy drivers with userspace modesetting,
> the original test remains in drm_wait_vblank_ioctl().
>
> v2:
> * keep the old test for legacy drivers in
> drm_wait_vblank_ioctl() (Daniel)
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/drm_irq.c| 10 +++---
> drivers/gpu/drm/drm_vblank.c | 13 +
> 2 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index c3bd664ea733..1d7785721323 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -74,10 +74,8 @@
> * only supports devices with a single interrupt on the main device stored in
> * _device.dev and set as the device paramter in drm_dev_alloc().
> *
> - * These IRQ helpers are strictly optional. Drivers which roll their own only
> - * need to set _device.irq_enabled to signal the DRM core that vblank
> - * interrupts are working. Since these helpers don't automatically clean up
> the
> - * requested interrupt like e.g. devm_request_irq() they're not really
> + * These IRQ helpers are strictly optional. Since these helpers don't
> automatically
> + * clean up the requested interrupt like e.g. devm_request_irq() they're not
> really
> * recommended.
> */
>
> @@ -91,9 +89,7 @@
> * and after the installation.
> *
> * This is the simplified helper interface provided for drivers with no
> special
> - * needs. Drivers which need to install interrupt handlers for multiple
> - * interrupts must instead set _device.irq_enabled to signal the DRM core
> - * that vblank interrupts are available.
> + * needs.
> *
> * @irq must match the interrupt number that would be passed to
> request_irq(),
> * if called directly instead of using this helper function.
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 3417e1ac7918..a98a4aad5037 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -1748,8 +1748,13 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void
> *data,
> unsigned int pipe_index;
> unsigned int flags, pipe, high_pipe;
>
> - if (!dev->irq_enabled)
> - return -EOPNOTSUPP;
> + if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> + if (!drm_dev_has_vblank(dev))
> + return -EOPNOTSUPP;
> + } else {
> + if (!dev->irq_enabled)
> + return -EOPNOTSUPP;
> + }
For a system call that is used quite a lot by userspace we have increased the
code size
in a noticeable way. Can we not cache it privately?
Best regards,
Liviu
>
> if (vblwait->request.type & _DRM_VBLANK_SIGNAL)
> return -EINVAL;
> @@ -2023,7 +2028,7 @@ int drm_crtc_get_sequence_ioctl(struct drm_device *dev,
> void *data,
> if (!drm_core_check_feature(dev, DRIVER_MODESET))
> return -EOPNOTSUPP;
>
> - if (!dev->irq_enabled)
> + if (!drm_dev_has_vblank(dev))
> return -EOPNOTSUPP;
>
> crtc = drm_crtc_find(dev, file_priv, get_seq->crtc_id);
> @@ -2082,7 +2087,7 @@ int drm_crtc_queue_sequence_ioctl(struct drm_device
> *dev, void *data,
> if (!drm_core_check_feature(dev, DRIVER_MODESET))
> return -EOPNOTSUPP;
>
> - if (!dev->irq_enabled)
> + if (!drm_dev_has_vblank(dev))
> return -EOPNOTSUPP;
>
> crtc = drm_crtc_find(dev, file_priv, queue_seq->crtc_id);
> --
> 2.32.0
>
--
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---
¯\_(ツ)_/¯
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/gamma: Clarify gamma lut uapi
On Fri, Mar 29, 2019 at 10:20:27AM +0100, Daniel Vetter wrote:
> Interpreting it as a 0.16 fixed point means we can't accurately
> represent 1.0. Which is one of the values we really should be able to
> represent.
>
> Since most (all?) luts have lower precision this will only affect
> rounding of 0x.
>
> Cc: Uma Shankar
> Cc: Ville Syrjälä
> Cc: Shashank Sharma
> Cc: "Kumar, Kiran S"
> Cc: Kausal Malladi
> Cc: Lionel Landwerlin
> Cc: Matt Roper
> Cc: Rob Bradford
> Cc: Daniel Stone
> Cc: Stefan Schake
> Cc: Eric Anholt
> Cc: Maarten Lankhorst
> Cc: Harry Wentland
> Cc: Leo Li
> Cc: amd-gfx@lists.freedesktop.org
> Cc: James (Qian) Wang
> Cc: Liviu Dudau
> Cc: Mali DP Maintainers
> Cc: CK Hu
> Cc: Philipp Zabel
> Cc: Yannick Fertre
> Cc: Philippe Cornu
> Cc: Benjamin Gaignard
> Cc: Vincent Abriou
> Cc: Tomi Valkeinen
> Cc: Boris Brezillon
> Signed-off-by: Daniel Vetter Signed-off-by: Daniel
> Vetter
Acked-by: Liviu Dudau
Best regards,
Liviu
> ---
> include/uapi/drm/drm_mode.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 09d72966899a..83cd1636b9be 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -621,7 +621,8 @@ struct drm_color_ctm {
>
> struct drm_color_lut {
> /*
> - * Data is U0.16 fixed point format.
> + * Values are mapped linearly to 0.0 - 1.0 range, with 0x0 == 0.0 and
> + * 0x == 1.0.
>*/
> __u16 red;
> __u16 green;
> --
> 2.20.1
>
--
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---
¯\_(ツ)_/¯
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2] drm: prefix header search paths with $(srctree)/
On Fri, Mar 29, 2019 at 08:32:41PM +0900, Masahiro Yamada wrote:
> Currently, the Kbuild core manipulates header search paths in a crazy
> way [1].
>
> To fix this mess, I want all Makefiles to add explicit $(srctree)/ to
> the search paths in the srctree. Some Makefiles are already written in
> that way, but not all. The goal of this work is to make the notation
> consistent, and finally get rid of the gross hacks.
>
> Having whitespaces after -I does not matter since commit 48f6e3cf5bc6
> ("kbuild: do not drop -I without parameter").
>
> [1]: https://patchwork.kernel.org/patch/9632347/
>
> Signed-off-by: Masahiro Yamada
> Reviewed-by: Sam Ravnborg
> ---
>
> I put all gpu/drm changes into a single patch because
> they are trivial conversion.
>
> If you are interested in the big picture of this work,
> the full patch set is available at the following URL.
>
> git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git
> build-test
>
>
> Changes in v2:
> - fix up the new driver komeda
> - Add Sam's Reviewed-by
>
> drivers/gpu/drm/amd/amdgpu/Makefile | 2 +-
> drivers/gpu/drm/amd/lib/Makefile| 2 +-
> drivers/gpu/drm/arm/display/komeda/Makefile | 4 ++--
> drivers/gpu/drm/i915/gvt/Makefile | 2 +-
> drivers/gpu/drm/msm/Makefile| 6 +++---
> drivers/gpu/drm/nouveau/Kbuild | 8
> 6 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile
> b/drivers/gpu/drm/amd/amdgpu/Makefile
> index 466da59..62bf9da 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> @@ -23,7 +23,7 @@
> # Makefile for the drm device driver. This driver provides support for the
> # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
>
> -FULL_AMD_PATH=$(src)/..
> +FULL_AMD_PATH=$(srctree)/$(src)/..
> DISPLAY_FOLDER_NAME=display
> FULL_AMD_DISPLAY_PATH = $(FULL_AMD_PATH)/$(DISPLAY_FOLDER_NAME)
>
> diff --git a/drivers/gpu/drm/amd/lib/Makefile
> b/drivers/gpu/drm/amd/lib/Makefile
> index 6902430..d534992 100644
> --- a/drivers/gpu/drm/amd/lib/Makefile
> +++ b/drivers/gpu/drm/amd/lib/Makefile
> @@ -27,6 +27,6 @@
> # driver components or later moved to kernel/lib for sharing with
> # other drivers.
>
> -ccflags-y := -I$(src)/../include
> +ccflags-y := -I $(srctree)/$(src)/../include
>
> obj-$(CONFIG_CHASH) += chash.o
> diff --git a/drivers/gpu/drm/arm/display/komeda/Makefile
> b/drivers/gpu/drm/arm/display/komeda/Makefile
> index 1b875e5..a72e30c 100644
> --- a/drivers/gpu/drm/arm/display/komeda/Makefile
> +++ b/drivers/gpu/drm/arm/display/komeda/Makefile
> @@ -1,8 +1,8 @@
> # SPDX-License-Identifier: GPL-2.0
>
> ccflags-y := \
> - -I$(src)/../include \
> - -I$(src)
> + -I $(srctree)/$(src)/../include \
> + -I $(srctree)/$(src)
>
> komeda-y := \
> komeda_drv.o \
For komeda:
Acked-by: Liviu Dudau
I'm assuming the series in going to be merged into one go, so I don't
have to take the individual patch in my tree, but if I'm wrong please
let me know.
Best regards,
Liviu
> diff --git a/drivers/gpu/drm/i915/gvt/Makefile
> b/drivers/gpu/drm/i915/gvt/Makefile
> index 271fb46..ea8324a 100644
> --- a/drivers/gpu/drm/i915/gvt/Makefile
> +++ b/drivers/gpu/drm/i915/gvt/Makefile
> @@ -5,5 +5,5 @@ GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o
> trace_points.o firmware.o \
> execlist.o scheduler.o sched_policy.o mmio_context.o cmd_parser.o
> debugfs.o \
> fb_decoder.o dmabuf.o page_track.o
>
> -ccflags-y+= -I$(src) -I$(src)/$(GVT_DIR)
> +ccflags-y+= -I $(srctree)/$(src) -I
> $(srctree)/$(src)/$(GVT_DIR)/
> i915-y += $(addprefix $(GVT_DIR)/,
> $(GVT_SOURCE))
> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
> index 56a70c7..b7b1ebd 100644
> --- a/drivers/gpu/drm/msm/Makefile
> +++ b/drivers/gpu/drm/msm/Makefile
> @@ -1,7 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0
> -ccflags-y := -Idrivers/gpu/drm/msm
> -ccflags-y += -Idrivers/gpu/drm/msm/disp/dpu1
> -ccflags-$(CONFIG_DRM_MSM_DSI) += -Idrivers/gpu/drm/msm/dsi
> +ccflags-y := -I $(srctree)/$(src)
> +ccflags-y += -I $(srctree)/$(src)/disp/dpu1
> +ccflags-$(CONFIG_DRM_MSM_DSI) += -I $(srctree)/$(src)/dsi
>
> msm-y := \
> adreno/adreno_device.o \
> diff --git a/drivers/gpu/drm/nouveau/Kbuild b/drivers/gpu/drm/nouveau/Kbuild
> index ea3035e..4fae728 100644
> --- a/drivers/gpu/drm/nouveau/Kbuild
> +++ b/drivers/gpu/drm/nouveau/Kbuild
> @@ -1,7 +1,7 @@
>
Re: [PATCH] drm: return false in drm_arch_can_wc_memory() for ARM
On Thu, Dec 20, 2018 at 04:36:19PM +0100, Daniel Vetter wrote: > On Thu, Dec 20, 2018 at 09:56:57AM -0500, Alex Deucher wrote: > > I'm not familiar enough with ARM to know if write combining > > is actually an architectural limitation or if it's an issue > > with the PCIe IPs used on various platforms, but so far > > everyone that has tried to run radeon hardware on > > ARM has had to disable it. So let's just make it official. > > wc on arm is Really Complicated (tm) afaiui. There's issues with aliasing > mappings and stuff, so you need to allocate your wc memory from special > pools. So probably best to just disable it until we figure this out. I believe both of you are conflating different issues under the wrong name. Write combining happens all the time with Arm, the ARMv8 architecture is a weakly-ordered model of memory so hardware is allowed to re-order or combine memory access as they seem fit. A while ago I did run an AMD GPU card on my Juno dev board and it worked (for a very limited definition of worked, I've only validated the fact that I could get an fbcon and could run un-accelerated X11). So I would be interested if Alex could share some of the scenarios where people are seeing failures. As for aliasing, yeah, having multiple aliases to the same piece of memory is a bad thing. The problem arises when devices on the PCI bus have memory allocated as device memory (which on Arm is non-cacheable and non-reorderable), but the PCI bus effectively acts as a write-combiner which changes the order of transactions. Therefore, for devices that have local memory associated with them (i.e. more than just register accesses) one should allocate memory in the first place that is Device-GRE (gathering, reordering and early-access). Otherwise, problems will surface that are not visible on x86 as that is a strongly ordered architecture. > > > Signed-off-by: Alex Deucher > > Reviewed-by: Daniel Vetter Given that this API is only used by AMD I'm OK for now with the change, but I think in general it is misleading and we should work towards fixing radeon and amd drivers. Best regards, Liviu > > > --- > > include/drm/drm_cache.h | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/include/drm/drm_cache.h b/include/drm/drm_cache.h > > index bfe1639df02d..691b4c4b0587 100644 > > --- a/include/drm/drm_cache.h > > +++ b/include/drm/drm_cache.h > > @@ -47,6 +47,8 @@ static inline bool drm_arch_can_wc_memory(void) > > return false; > > #elif defined(CONFIG_MIPS) && defined(CONFIG_CPU_LOONGSON3) > > return false; > > +#elif defined(CONFIG_ARM) || defined(CONFIG_ARM64) > > + return false; > > #else > > return true; > > #endif > > -- > > 2.13.6 > > > > ___ > > dri-devel mailing list > > dri-de...@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ツ)_/¯ ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 2/2] drm/atomic: Create and use __drm_atomic_helper_crtc_reset() everywhere
On Mon, Nov 12, 2018 at 04:01:14PM +0100, Maarten Lankhorst wrote:
> We already have __drm_atomic_helper_connector_reset() and
> __drm_atomic_helper_plane_reset(), extend this to crtc as well.
>
> Most drivers already have a gpu reset hook, correct it.
> Nouveau already implemented its own __drm_atomic_helper_crtc_reset(),
> convert it to the common one.
>
> Signed-off-by: Maarten Lankhorst
> Cc: Harry Wentland
> Cc: Leo Li
> Cc: Alex Deucher
> Cc: "Christian König"
> Cc: "David (ChunMing) Zhou"
> Cc: David Airlie
> Cc: Liviu Dudau
> Cc: Brian Starkey
> Cc: Mali DP Maintainers
> Cc: Boris Brezillon
> Cc: Nicolas Ferre
> Cc: Alexandre Belloni
> Cc: Ludovic Desroches
> Cc: Maarten Lankhorst
> Cc: Maxime Ripard
> Cc: Sean Paul
> Cc: Jani Nikula
> Cc: Joonas Lahtinen
> Cc: Rodrigo Vivi
> Cc: Philipp Zabel
> Cc: CK Hu
> Cc: Matthias Brugger
> Cc: Rob Clark
> Cc: Ben Skeggs
> Cc: Tomi Valkeinen
> Cc: Laurent Pinchart
> Cc: Kieran Bingham
> Cc: Sandy Huang
> Cc: "Heiko Stübner"
> Cc: Thierry Reding
> Cc: Jonathan Hunter
> Cc: Eric Anholt
> Cc: VMware Graphics
> Cc: Sinclair Yeh
> Cc: Thomas Hellstrom
> Cc: Tony Cheng
> Cc: Shirish S
> Cc: Mikita Lipski
> Cc: Bhawanpreet Lakha
> Cc: David Francis
> Cc: Anthony Koo
> Cc: Jeykumar Sankaran
> Cc: Jordan Crouse
> Cc: Bruce Wang
> Cc: Sravanthi Kollukuduru
> Cc: Archit Taneja
> Cc: Steve Kowalik
> Cc: Carsten Behling
> Cc: Haneen Mohammed
> Cc: Daniel Vetter
> Cc: Rodrigo Siqueira
> Cc: Mahesh Kumar
> Cc: amd-gfx@lists.freedesktop.org
> Cc: dri-de...@lists.freedesktop.org
> Cc: linux-ker...@vger.kernel.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: intel-...@lists.freedesktop.org
> Cc: linux-media...@lists.infradead.org
> Cc: linux-arm-...@vger.kernel.org
> Cc: freedr...@lists.freedesktop.org
> Cc: nouv...@lists.freedesktop.org
> Cc: linux-renesas-...@vger.kernel.org
> Cc: linux-rockc...@lists.infradead.org
> Cc: linux-te...@vger.kernel.org
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 +--
> drivers/gpu/drm/arm/malidp_crtc.c | 5 +--
> .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c| 5 +--
> drivers/gpu/drm/drm_atomic_state_helper.c | 31 ---
> drivers/gpu/drm/i915/intel_display.c | 2 +-
> drivers/gpu/drm/imx/ipuv3-crtc.c | 5 +--
> drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 5 +--
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 12 ++-
> drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 6 +---
> drivers/gpu/drm/nouveau/dispnv50/head.c | 13 ++--
> drivers/gpu/drm/omapdrm/omap_crtc.c | 7 ++---
> drivers/gpu/drm/rcar-du/rcar_du_crtc.c| 4 +--
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 7 +++--
> drivers/gpu/drm/tegra/dc.c| 5 +--
> drivers/gpu/drm/vc4/vc4_crtc.c| 8 ++---
> drivers/gpu/drm/vkms/vkms_crtc.c | 7 +
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 9 +-
> include/drm/drm_atomic_state_helper.h | 2 ++
> 18 files changed, 56 insertions(+), 81 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 5064768642f3..770a71726cd1 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2802,9 +2802,7 @@ static void dm_crtc_reset_state(struct drm_crtc *crtc)
> if (WARN_ON(!state))
> return;
>
> - crtc->state = >base;
> - crtc->state->crtc = crtc;
> -
> + __drm_atomic_helper_crtc_reset(crtc, >base);
> }
>
> static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/arm/malidp_crtc.c
> b/drivers/gpu/drm/arm/malidp_crtc.c
> index e1b72782848c..9a924ff27148 100644
> --- a/drivers/gpu/drm/arm/malidp_crtc.c
> +++ b/drivers/gpu/drm/arm/malidp_crtc.c
> @@ -474,10 +474,7 @@ static void malidp_crtc_reset(struct drm_crtc *crtc)
>
> kfree(state);
> state = kzalloc(sizeof(*state), GFP_KERNEL);
> - if (state) {
> - crtc->state = >base;
> - crtc->state->crtc = crtc;
> - }
> + __drm_atomic_helper_crtc_reset(crtc, >base);
> }
>
> static void malidp_crtc_destroy_state(struct drm_crtc *crtc,
For the malidp changes:
Acked-by: Liviu Dudau
Best regards,
Liviu
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index 96f4082671fe..8084d549c7d1 100644
> --- a/drivers/gpu/
Re: [PATCH] RFC: Make igts for cross-driver stuff mandatory?
On Fri, Oct 19, 2018 at 10:50:49AM +0200, Daniel Vetter wrote:
> Hi all,
Hi,
(Replying from my personal address as the work email seems to have let
this one go to /dev/null)
>
> This is just to collect feedback on this idea, and see whether the
> overall dri-devel community stands on all this. I think the past few
> cross-vendor uapi extensions all came with igts attached, and
> personally I think there's lots of value in having them: A
> cross-vendor interface isn't useful if every driver implements it
> slightly differently.
I would like to also get some clarification on where we are standing on
"tests vs 'real code'" stanza. Does making igt tests mandatory replace
the need for 'real code' or does it add to the list of requirements? If
the later, then I think the bar rises in terms of showing igts'
usefulness / benefits.
>
> I think there's 2 questions here:
>
> - Do we want to make such testcases mandatory?
I'm a bit reluctant to make it so by fiat. I think that showing the
usefulness of having igts tests to newcomers (by adding with this patch
some more information about why IGT is a good place to add your testing to)
and getting more mature drivers to get tested under IGT on a regular basis
would make adoption of IGT for testing a community standard.
>
> - If yes, are we there yet, or is there something crucially missing
> still?
I'm just getting back into IGT by refreshing the writeback patches, but
by looking at the commit log I get the impression that there aren't that
many patches that target drivers other than Intel's. Are all the non-Intel
patches so generic that one doesn't need to specify a target driver for
those changes (in which case great, but then why is Intel's so different?),
or are the others not bothered with IGT support?
At the moment I'm a bit on the fence on this. Not having spent too much
time with IGT in the last 6 months, I'm probably closer to a newcomer in
my attitude towards IGT and at the moment I'm not clear on how to answer the
"Why?" and "What is in it for me?" questions.
Best regards,
Liviu
>
> And of course there's a bunch of details to figure out. Like we
> probably want to also recommend the selftests/unit-tests in
> drivers/gpu/drm/selftest, since fairly often that's a much more
> effective approach to checking the details than from userspace.
>
> Feedback and thoughts very much appreciated.
>
> Cheers, Daniel
> ---
> Documentation/gpu/drm-uapi.rst | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
> index 4b4bf2c5eac5..91cf6e4b6303 100644
> --- a/Documentation/gpu/drm-uapi.rst
> +++ b/Documentation/gpu/drm-uapi.rst
> @@ -238,6 +238,13 @@ DRM specific patterns. Note that ENOTTY has the slightly
> unintuitive meaning of
> Testing and validation
> ==
>
> +Testing Requirements for userspace API
> +--
> +
> +New cross-driver userspace interface extensions, like new IOCTL, new KMS
> +properties, new files in sysfs or anything else that constitutes an API
> change
> +need to have driver-agnostic testcases in IGT for that feature.
> +
> Validating changes with IGT
> ---
>
> --
> 2.19.1
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
/`\
/ : |
_.._ | '/
/`\| /
| .-._ '-"` (
|_/ / o o\
| == () ==
\ -- / __
/ ---<_ |
|___
| \\ \ | I would like to fix the world but |
/
| | \\__ \ | no one gives me the source code. |
/
/ ; |.__) / |__|
\
(_/.-. ; /__)
(_\
{ `| \_/
'-\ / |
| / |
/ \ '-.
\__|-'
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Don't save new cursor size before updating CUR_SIZE register.
On Fri, Dec 16, 2016 at 07:16:25PM +, Deucher, Alexander wrote:
> > -Original Message-
> > From: Liviu Dudau [mailto:li...@dudau.co.uk]
> > Sent: Friday, December 16, 2016 2:11 PM
> > To: Daenzer, Michel; Deucher, Alexander
> > Cc: Koenig, Christian; David Airlie; dri-de...@lists.freedesktop.org; amd-
> > g...@lists.freedesktop.org; Liviu Dudau
> > Subject: [PATCH] drm/amdgpu: Don't save new cursor size before updating
> > CUR_SIZE register.
> >
> > Commit 7c83d7abc999 ("drm/amdgpu: Only update the CUR_SIZE register
> > when
> > necessary") did not cleanup correctly the old code for DCE v6 and v8.
> > As a consequence, cursor updates stopped working for my Radeon R9
> > 1002:67b0
> > dual-monitor setup.
> >
> > Fixes: 7c83d7abc999 ("drm/amdgpu: Only update the CUR_SIZE register
> > when necessary")
> > Signed-off-by: Liviu Dudau <li...@dudau.co.uk>
>
> Already fixed:
> https://lists.freedesktop.org/archives/amd-gfx/2016-December/003985.html
Sorry, I've only checked the dri-devel list, I've only noticed there is an
amd-gfx one while preparing the patch for sending.
> and queued for fixes:
> https://cgit.freedesktop.org/~agd5f/linux/commit/?h=drm-next-4.10-wip=837b2d51a5847584fe64aebbc94ef8b7ae59fd87
Cool, thanks!
Best regards,
Liviu
>
> Alex
>
> > ---
> > drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 6 +-
> > drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 --
> > 2 files changed, 1 insertion(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > index e564442..b4e4ec6 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > @@ -1944,9 +1944,7 @@ static int dce_v6_0_crtc_cursor_set2(struct
> > drm_crtc *crtc,
> >
> > dce_v6_0_lock_cursor(crtc, true);
> >
> > - if (width != amdgpu_crtc->cursor_width ||
> > - height != amdgpu_crtc->cursor_height ||
> > - hot_x != amdgpu_crtc->cursor_hot_x ||
> > + if (hot_x != amdgpu_crtc->cursor_hot_x ||
> > hot_y != amdgpu_crtc->cursor_hot_y) {
> > int x, y;
> >
> > @@ -1955,8 +1953,6 @@ static int dce_v6_0_crtc_cursor_set2(struct
> > drm_crtc *crtc,
> >
> > dce_v6_0_cursor_move_locked(crtc, x, y);
> >
> > - amdgpu_crtc->cursor_width = width;
> > - amdgpu_crtc->cursor_height = height;
> > amdgpu_crtc->cursor_hot_x = hot_x;
> > amdgpu_crtc->cursor_hot_y = hot_y;
> > }
> > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> > b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> > index 6ce7fb4..584abe8 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> > @@ -2438,8 +2438,6 @@ static int dce_v8_0_crtc_cursor_set2(struct
> > drm_crtc *crtc,
> >
> > dce_v8_0_cursor_move_locked(crtc, x, y);
> >
> > - amdgpu_crtc->cursor_width = width;
> > - amdgpu_crtc->cursor_height = height;
> > amdgpu_crtc->cursor_hot_x = hot_x;
> > amdgpu_crtc->cursor_hot_y = hot_y;
> > }
> > --
> > 2.10.2
>
--
---
.oooO
( )
\ ( Oooo.
\_) ( )
) /
(_/
One small step
for me ...
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: Don't save new cursor size before updating CUR_SIZE register.
Commit 7c83d7abc999 ("drm/amdgpu: Only update the CUR_SIZE register when
necessary") did not cleanup correctly the old code for DCE v6 and v8.
As a consequence, cursor updates stopped working for my Radeon R9 1002:67b0
dual-monitor setup.
Fixes: 7c83d7abc999 ("drm/amdgpu: Only update the CUR_SIZE register when
necessary")
Signed-off-by: Liviu Dudau <li...@dudau.co.uk>
---
drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 6 +-
drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 --
2 files changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
index e564442..b4e4ec6 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
@@ -1944,9 +1944,7 @@ static int dce_v6_0_crtc_cursor_set2(struct drm_crtc
*crtc,
dce_v6_0_lock_cursor(crtc, true);
- if (width != amdgpu_crtc->cursor_width ||
- height != amdgpu_crtc->cursor_height ||
- hot_x != amdgpu_crtc->cursor_hot_x ||
+ if (hot_x != amdgpu_crtc->cursor_hot_x ||
hot_y != amdgpu_crtc->cursor_hot_y) {
int x, y;
@@ -1955,8 +1953,6 @@ static int dce_v6_0_crtc_cursor_set2(struct drm_crtc
*crtc,
dce_v6_0_cursor_move_locked(crtc, x, y);
- amdgpu_crtc->cursor_width = width;
- amdgpu_crtc->cursor_height = height;
amdgpu_crtc->cursor_hot_x = hot_x;
amdgpu_crtc->cursor_hot_y = hot_y;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
index 6ce7fb4..584abe8 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
@@ -2438,8 +2438,6 @@ static int dce_v8_0_crtc_cursor_set2(struct drm_crtc
*crtc,
dce_v8_0_cursor_move_locked(crtc, x, y);
- amdgpu_crtc->cursor_width = width;
- amdgpu_crtc->cursor_height = height;
amdgpu_crtc->cursor_hot_x = hot_x;
amdgpu_crtc->cursor_hot_y = hot_y;
}
--
2.10.2
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
