[RFC/PATCH] drm: Make the .mode_fixup() operations mode argument a const pointer
On Wed, May 30, 2012 at 12:11:49PM +0200, Laurent Pinchart wrote: > Hi Daniel, > > On Wednesday 30 May 2012 12:02:19 Daniel Vetter wrote: > > On Wed, May 30, 2012 at 11:24:50AM +0200, Laurent Pinchart wrote: > > > On Wednesday 30 May 2012 11:18:56 Daniel Vetter wrote: > > > > On Wed, May 30, 2012 at 01:01:08AM +0200, Laurent Pinchart wrote: > > > > > The passed mode must not be modified by the operation, make it const. > > > > > > > > > > Signed-off-by: Laurent Pinchart > > > > > > > > Acked-by: Daniel Vetter > > > > > > Thank you for the ack. Would you also be so kind as to tell me how to fix > > > the related i915 issue (as briefly stated in the patch, the i915 driver > > > modifies mode->clock in intel_dp_mode_fixup()) ? Or, even better, provide > > > a patch for that ? :-) > > > > Oops, lack of coffee. Patch is in the works. > > No worries :-) As my patch should go on top of yours, could you please take > it > in your tree ? Well I think that patch should go through Dave's tree directly. But I send him regular -next pull requests, so that should stall your patch for long. -Daniel > > Btw, do you plan to constify other stuff? > > Not immediately. When I tried to implement .mode_fixup() I wasn't sure > whether > drivers were allowed to modify the mode argument. After researching it I came > up to the conclusion that they should not, hence the patch. It would make > sense to constify other structures and function arguments as well though. > > -- > Regards, > > Laurent Pinchart > -- Daniel Vetter Mail: daniel at ffwll.ch Mobile: +41 (0)79 365 57 48
[RFC/PATCH] drm: Make the .mode_fixup() operations mode argument a const pointer
Hi Daniel, On Wednesday 30 May 2012 12:02:19 Daniel Vetter wrote: > On Wed, May 30, 2012 at 11:24:50AM +0200, Laurent Pinchart wrote: > > On Wednesday 30 May 2012 11:18:56 Daniel Vetter wrote: > > > On Wed, May 30, 2012 at 01:01:08AM +0200, Laurent Pinchart wrote: > > > > The passed mode must not be modified by the operation, make it const. > > > > > > > > Signed-off-by: Laurent Pinchart > > > > > > Acked-by: Daniel Vetter > > > > Thank you for the ack. Would you also be so kind as to tell me how to fix > > the related i915 issue (as briefly stated in the patch, the i915 driver > > modifies mode->clock in intel_dp_mode_fixup()) ? Or, even better, provide > > a patch for that ? :-) > > Oops, lack of coffee. Patch is in the works. No worries :-) As my patch should go on top of yours, could you please take it in your tree ? > Btw, do you plan to constify other stuff? Not immediately. When I tried to implement .mode_fixup() I wasn't sure whether drivers were allowed to modify the mode argument. After researching it I came up to the conclusion that they should not, hence the patch. It would make sense to constify other structures and function arguments as well though. -- Regards, Laurent Pinchart
[RFC/PATCH] drm: Make the .mode_fixup() operations mode argument a const pointer
On Wed, May 30, 2012 at 11:24:50AM +0200, Laurent Pinchart wrote: > Hi Daniel, > > On Wednesday 30 May 2012 11:18:56 Daniel Vetter wrote: > > On Wed, May 30, 2012 at 01:01:08AM +0200, Laurent Pinchart wrote: > > > The passed mode must not be modified by the operation, make it const. > > > > > > Signed-off-by: Laurent Pinchart > > > > Acked-by: Daniel Vetter > > Thank you for the ack. Would you also be so kind as to tell me how to fix the > related i915 issue (as briefly stated in the patch, the i915 driver modifies > mode->clock in intel_dp_mode_fixup()) ? Or, even better, provide a patch for > that ? :-) Oops, lack of coffee. Patch is in the works. Btw, do you plan to constify other stuff? -Daneil -- Daniel Vetter Mail: daniel at ffwll.ch Mobile: +41 (0)79 365 57 48
[RFC/PATCH] drm: Make the .mode_fixup() operations mode argument a const pointer
Hi Daniel, On Wednesday 30 May 2012 11:18:56 Daniel Vetter wrote: > On Wed, May 30, 2012 at 01:01:08AM +0200, Laurent Pinchart wrote: > > The passed mode must not be modified by the operation, make it const. > > > > Signed-off-by: Laurent Pinchart > > Acked-by: Daniel Vetter Thank you for the ack. Would you also be so kind as to tell me how to fix the related i915 issue (as briefly stated in the patch, the i915 driver modifies mode->clock in intel_dp_mode_fixup()) ? Or, even better, provide a patch for that ? :-) -- Regards, Laurent Pinchart
[RFC/PATCH] drm: Make the .mode_fixup() operations mode argument a const pointer
On Wed, May 30, 2012 at 01:01:08AM +0200, Laurent Pinchart wrote: > The passed mode must not be modified by the operation, make it const. > > Signed-off-by: Laurent Pinchart Acked-by: Daniel Vetter -- Daniel Vetter Mail: daniel at ffwll.ch Mobile: +41 (0)79 365 57 48
[RFC/PATCH] drm: Make the .mode_fixup() operations mode argument a const pointer
The passed mode must not be modified by the operation, make it const.
Signed-off-by: Laurent Pinchart
---
This will break the i915 driver, as it modifies mode->clock in
intel_dp_mode_fixup(), hence the RFC state. Is this incorrect behaviour from
the i915 driver ?
drivers/gpu/drm/exynos/exynos_drm_crtc.c|2 +-
drivers/gpu/drm/exynos/exynos_drm_drv.h |2 +-
drivers/gpu/drm/exynos/exynos_drm_encoder.c |2 +-
drivers/gpu/drm/exynos/exynos_drm_hdmi.c|2 +-
drivers/gpu/drm/exynos/exynos_drm_hdmi.h|2 +-
drivers/gpu/drm/exynos/exynos_hdmi.c|2 +-
drivers/gpu/drm/gma500/cdv_intel_crt.c |2 +-
drivers/gpu/drm/gma500/cdv_intel_display.c |2 +-
drivers/gpu/drm/gma500/cdv_intel_hdmi.c |2 +-
drivers/gpu/drm/gma500/cdv_intel_lvds.c |2 +-
drivers/gpu/drm/gma500/mdfld_dsi_dpi.c |2 +-
drivers/gpu/drm/gma500/mdfld_dsi_dpi.h |2 +-
drivers/gpu/drm/gma500/mdfld_intel_display.c|2 +-
drivers/gpu/drm/gma500/oaktrail_crtc.c |2 +-
drivers/gpu/drm/gma500/oaktrail_hdmi.c |2 +-
drivers/gpu/drm/gma500/psb_intel_display.c |2 +-
drivers/gpu/drm/gma500/psb_intel_drv.h |2 +-
drivers/gpu/drm/gma500/psb_intel_lvds.c |2 +-
drivers/gpu/drm/gma500/psb_intel_sdvo.c |6 +++---
drivers/gpu/drm/i2c/ch7006_drv.c|2 +-
drivers/gpu/drm/i2c/ch7006_mode.c |2 +-
drivers/gpu/drm/i2c/sil164_drv.c|2 +-
drivers/gpu/drm/i915/dvo.h |2 +-
drivers/gpu/drm/i915/intel_crt.c|2 +-
drivers/gpu/drm/i915/intel_dp.c |3 ++-
drivers/gpu/drm/i915/intel_drv.h|2 +-
drivers/gpu/drm/i915/intel_dvo.c|2 +-
drivers/gpu/drm/i915/intel_hdmi.c |2 +-
drivers/gpu/drm/i915/intel_lvds.c |2 +-
drivers/gpu/drm/i915/intel_panel.c |2 +-
drivers/gpu/drm/i915/intel_sdvo.c |6 +++---
drivers/gpu/drm/i915/intel_tv.c |3 ++-
drivers/gpu/drm/nouveau/nv04_crtc.c |2 +-
drivers/gpu/drm/nouveau/nv04_dac.c |2 +-
drivers/gpu/drm/nouveau/nv04_dfp.c |2 +-
drivers/gpu/drm/nouveau/nv17_tv.c |2 +-
drivers/gpu/drm/nouveau/nv50_crtc.c |2 +-
drivers/gpu/drm/nouveau/nv50_dac.c |3 ++-
drivers/gpu/drm/nouveau/nv50_sor.c |3 ++-
drivers/gpu/drm/nouveau/nvd0_display.c |8 +---
drivers/gpu/drm/radeon/atombios_crtc.c |2 +-
drivers/gpu/drm/radeon/atombios_dp.c|2 +-
drivers/gpu/drm/radeon/atombios_encoders.c |4 ++--
drivers/gpu/drm/radeon/radeon_display.c |4 ++--
drivers/gpu/drm/radeon/radeon_legacy_crtc.c |2 +-
drivers/gpu/drm/radeon/radeon_legacy_encoders.c |2 +-
drivers/gpu/drm/radeon/radeon_mode.h|4 ++--
drivers/gpu/drm/udl/udl_encoder.c |2 +-
drivers/gpu/drm/udl/udl_modeset.c |2 +-
drivers/staging/omapdrm/omap_crtc.c |2 +-
drivers/staging/omapdrm/omap_encoder.c |2 +-
include/drm/drm_crtc_helper.h |4 ++--
52 files changed, 68 insertions(+), 62 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 4afb625..32a34c8 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -237,7 +237,7 @@ static void exynos_drm_crtc_commit(struct drm_crtc *crtc)
static bool
exynos_drm_crtc_mode_fixup(struct drm_crtc *crtc,
- struct drm_display_mode *mode,
+ const struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode)
{
DRM_DEBUG_KMS("%s\n", __FILE__);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h
b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index c82c90c..277653d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -174,7 +174,7 @@ struct exynos_drm_manager_ops {
void (*apply)(struct device *subdrv_dev);
void (*mode_fixup)(struct device *subdrv_dev,
struct drm_connector *connector,
- struct drm_display_mode *mode,
+ const struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode);
void (*mode_set)(struct device *subdrv_dev, void *mode);
void (*get_max_resol)(struct device *subdrv_dev, unsigned int *width,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_encoder.c
b/drivers/gpu/drm/exynos/exynos_drm_encoder.c
index 23d5ad3..4a13a74 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_encoder.c
+++
Re: [RFC/PATCH] drm: Make the .mode_fixup() operations mode argument a const pointer
On Wed, May 30, 2012 at 01:01:08AM +0200, Laurent Pinchart wrote: The passed mode must not be modified by the operation, make it const. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Acked-by: Daniel Vetter daniel.vet...@ffwll.ch -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC/PATCH] drm: Make the .mode_fixup() operations mode argument a const pointer
Hi Daniel, On Wednesday 30 May 2012 11:18:56 Daniel Vetter wrote: On Wed, May 30, 2012 at 01:01:08AM +0200, Laurent Pinchart wrote: The passed mode must not be modified by the operation, make it const. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Acked-by: Daniel Vetter daniel.vet...@ffwll.ch Thank you for the ack. Would you also be so kind as to tell me how to fix the related i915 issue (as briefly stated in the patch, the i915 driver modifies mode-clock in intel_dp_mode_fixup()) ? Or, even better, provide a patch for that ? :-) -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC/PATCH] drm: Make the .mode_fixup() operations mode argument a const pointer
On Wed, May 30, 2012 at 11:24:50AM +0200, Laurent Pinchart wrote: Hi Daniel, On Wednesday 30 May 2012 11:18:56 Daniel Vetter wrote: On Wed, May 30, 2012 at 01:01:08AM +0200, Laurent Pinchart wrote: The passed mode must not be modified by the operation, make it const. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Acked-by: Daniel Vetter daniel.vet...@ffwll.ch Thank you for the ack. Would you also be so kind as to tell me how to fix the related i915 issue (as briefly stated in the patch, the i915 driver modifies mode-clock in intel_dp_mode_fixup()) ? Or, even better, provide a patch for that ? :-) Oops, lack of coffee. Patch is in the works. Btw, do you plan to constify other stuff? -Daneil -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC/PATCH] drm: Make the .mode_fixup() operations mode argument a const pointer
Hi Daniel, On Wednesday 30 May 2012 12:02:19 Daniel Vetter wrote: On Wed, May 30, 2012 at 11:24:50AM +0200, Laurent Pinchart wrote: On Wednesday 30 May 2012 11:18:56 Daniel Vetter wrote: On Wed, May 30, 2012 at 01:01:08AM +0200, Laurent Pinchart wrote: The passed mode must not be modified by the operation, make it const. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Acked-by: Daniel Vetter daniel.vet...@ffwll.ch Thank you for the ack. Would you also be so kind as to tell me how to fix the related i915 issue (as briefly stated in the patch, the i915 driver modifies mode-clock in intel_dp_mode_fixup()) ? Or, even better, provide a patch for that ? :-) Oops, lack of coffee. Patch is in the works. No worries :-) As my patch should go on top of yours, could you please take it in your tree ? Btw, do you plan to constify other stuff? Not immediately. When I tried to implement .mode_fixup() I wasn't sure whether drivers were allowed to modify the mode argument. After researching it I came up to the conclusion that they should not, hence the patch. It would make sense to constify other structures and function arguments as well though. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC/PATCH] drm: Make the .mode_fixup() operations mode argument a const pointer
On Wed, May 30, 2012 at 12:11:49PM +0200, Laurent Pinchart wrote: Hi Daniel, On Wednesday 30 May 2012 12:02:19 Daniel Vetter wrote: On Wed, May 30, 2012 at 11:24:50AM +0200, Laurent Pinchart wrote: On Wednesday 30 May 2012 11:18:56 Daniel Vetter wrote: On Wed, May 30, 2012 at 01:01:08AM +0200, Laurent Pinchart wrote: The passed mode must not be modified by the operation, make it const. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Acked-by: Daniel Vetter daniel.vet...@ffwll.ch Thank you for the ack. Would you also be so kind as to tell me how to fix the related i915 issue (as briefly stated in the patch, the i915 driver modifies mode-clock in intel_dp_mode_fixup()) ? Or, even better, provide a patch for that ? :-) Oops, lack of coffee. Patch is in the works. No worries :-) As my patch should go on top of yours, could you please take it in your tree ? Well I think that patch should go through Dave's tree directly. But I send him regular -next pull requests, so that should stall your patch for long. -Daniel Btw, do you plan to constify other stuff? Not immediately. When I tried to implement .mode_fixup() I wasn't sure whether drivers were allowed to modify the mode argument. After researching it I came up to the conclusion that they should not, hence the patch. It would make sense to constify other structures and function arguments as well though. -- Regards, Laurent Pinchart -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC/PATCH] drm: Make the .mode_fixup() operations mode argument a const pointer
The passed mode must not be modified by the operation, make it const.
Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
This will break the i915 driver, as it modifies mode-clock in
intel_dp_mode_fixup(), hence the RFC state. Is this incorrect behaviour from
the i915 driver ?
drivers/gpu/drm/exynos/exynos_drm_crtc.c|2 +-
drivers/gpu/drm/exynos/exynos_drm_drv.h |2 +-
drivers/gpu/drm/exynos/exynos_drm_encoder.c |2 +-
drivers/gpu/drm/exynos/exynos_drm_hdmi.c|2 +-
drivers/gpu/drm/exynos/exynos_drm_hdmi.h|2 +-
drivers/gpu/drm/exynos/exynos_hdmi.c|2 +-
drivers/gpu/drm/gma500/cdv_intel_crt.c |2 +-
drivers/gpu/drm/gma500/cdv_intel_display.c |2 +-
drivers/gpu/drm/gma500/cdv_intel_hdmi.c |2 +-
drivers/gpu/drm/gma500/cdv_intel_lvds.c |2 +-
drivers/gpu/drm/gma500/mdfld_dsi_dpi.c |2 +-
drivers/gpu/drm/gma500/mdfld_dsi_dpi.h |2 +-
drivers/gpu/drm/gma500/mdfld_intel_display.c|2 +-
drivers/gpu/drm/gma500/oaktrail_crtc.c |2 +-
drivers/gpu/drm/gma500/oaktrail_hdmi.c |2 +-
drivers/gpu/drm/gma500/psb_intel_display.c |2 +-
drivers/gpu/drm/gma500/psb_intel_drv.h |2 +-
drivers/gpu/drm/gma500/psb_intel_lvds.c |2 +-
drivers/gpu/drm/gma500/psb_intel_sdvo.c |6 +++---
drivers/gpu/drm/i2c/ch7006_drv.c|2 +-
drivers/gpu/drm/i2c/ch7006_mode.c |2 +-
drivers/gpu/drm/i2c/sil164_drv.c|2 +-
drivers/gpu/drm/i915/dvo.h |2 +-
drivers/gpu/drm/i915/intel_crt.c|2 +-
drivers/gpu/drm/i915/intel_dp.c |3 ++-
drivers/gpu/drm/i915/intel_drv.h|2 +-
drivers/gpu/drm/i915/intel_dvo.c|2 +-
drivers/gpu/drm/i915/intel_hdmi.c |2 +-
drivers/gpu/drm/i915/intel_lvds.c |2 +-
drivers/gpu/drm/i915/intel_panel.c |2 +-
drivers/gpu/drm/i915/intel_sdvo.c |6 +++---
drivers/gpu/drm/i915/intel_tv.c |3 ++-
drivers/gpu/drm/nouveau/nv04_crtc.c |2 +-
drivers/gpu/drm/nouveau/nv04_dac.c |2 +-
drivers/gpu/drm/nouveau/nv04_dfp.c |2 +-
drivers/gpu/drm/nouveau/nv17_tv.c |2 +-
drivers/gpu/drm/nouveau/nv50_crtc.c |2 +-
drivers/gpu/drm/nouveau/nv50_dac.c |3 ++-
drivers/gpu/drm/nouveau/nv50_sor.c |3 ++-
drivers/gpu/drm/nouveau/nvd0_display.c |8 +---
drivers/gpu/drm/radeon/atombios_crtc.c |2 +-
drivers/gpu/drm/radeon/atombios_dp.c|2 +-
drivers/gpu/drm/radeon/atombios_encoders.c |4 ++--
drivers/gpu/drm/radeon/radeon_display.c |4 ++--
drivers/gpu/drm/radeon/radeon_legacy_crtc.c |2 +-
drivers/gpu/drm/radeon/radeon_legacy_encoders.c |2 +-
drivers/gpu/drm/radeon/radeon_mode.h|4 ++--
drivers/gpu/drm/udl/udl_encoder.c |2 +-
drivers/gpu/drm/udl/udl_modeset.c |2 +-
drivers/staging/omapdrm/omap_crtc.c |2 +-
drivers/staging/omapdrm/omap_encoder.c |2 +-
include/drm/drm_crtc_helper.h |4 ++--
52 files changed, 68 insertions(+), 62 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 4afb625..32a34c8 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -237,7 +237,7 @@ static void exynos_drm_crtc_commit(struct drm_crtc *crtc)
static bool
exynos_drm_crtc_mode_fixup(struct drm_crtc *crtc,
- struct drm_display_mode *mode,
+ const struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode)
{
DRM_DEBUG_KMS(%s\n, __FILE__);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h
b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index c82c90c..277653d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -174,7 +174,7 @@ struct exynos_drm_manager_ops {
void (*apply)(struct device *subdrv_dev);
void (*mode_fixup)(struct device *subdrv_dev,
struct drm_connector *connector,
- struct drm_display_mode *mode,
+ const struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode);
void (*mode_set)(struct device *subdrv_dev, void *mode);
void (*get_max_resol)(struct device *subdrv_dev, unsigned int *width,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_encoder.c
b/drivers/gpu/drm/exynos/exynos_drm_encoder.c
index 23d5ad3..4a13a74 100644
---
