Re: [PATCH 15/20] drm/radeon/r600d: Move 'rc600_*' prototypes into shared header
On Tue, 10 Nov 2020, Alex Deucher wrote: > On Tue, Nov 10, 2020 at 4:41 AM Lee Jones wrote: > > > > On Tue, 10 Nov 2020, Sam Ravnborg wrote: > > > > > Hi Lee, > > > > > > > > the *d.h headers are supposed to just be hardware definitions. I'd > > > > > prefer to keep driver stuff out of them. > > > > > > > > That's fine (I did wonder if that were the case). > > > > > > > > I need an answer from you and Sam whether I can create new headers. > > > > > > > > For me, it is the right thing to do. > > > > > > Please follow the advice of Alex for the radeon driver. > > > > Great. Thanks for resolving this Sam. > > > > Will fix all occurrences. > > I'm not really following these patch sets you are sending out. They > all seem completely independent. I was expecting updated versions > with feedback integrated, but they seem to be just different fixes. > Are you planning to send out updated versions based on feedback from > these series? Also, some of the patches have subtle errors in them > (e.g., wrong descriptions of variables, wrong copyright headers on new > files, etc.), do you mind if I fix them up locally when applying or > would you rather I point out the changes and you can integrate them > and resend? Apologies for any confusion. Let me try to shed some light. All 4 sets sent thus far have been independent. There are 5000 issues to solve in drivers/gpu - I'm trying my best to whittle them down. We're down to 2200 right now, so it seems to be going well. I'm aware that some of the patches have been accepted already, so the plan is to wait a few days, let them settle into -next, then start; rebasing sets, applying fix-ups and sending out v2s. FYI: There are also outstanding rebases due for; wireless, net, input and SCSI, as well as the 4 DRM sets. I'm getting to all of them. With regards to how you deal with incorrectness, that's entirely up to you. Feel free to either fix-up any issues you see or to provide review comments and let me deal with it. Whatever works for you. A note on copyrights on new files; the new files are copied directly from old, previously accepted/currently residing ones in order to keep in-line with what's expected of the subsystem. If the format has been updated and/or you would like them modernised, please either fix them up at your leisure or let me know what's required and I'll rework and resubmit them. -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH 15/20] drm/radeon/r600d: Move 'rc600_*' prototypes into shared header
On Tue, Nov 10, 2020 at 4:41 AM Lee Jones wrote: > > On Tue, 10 Nov 2020, Sam Ravnborg wrote: > > > Hi Lee, > > > > > > the *d.h headers are supposed to just be hardware definitions. I'd > > > > prefer to keep driver stuff out of them. > > > > > > That's fine (I did wonder if that were the case). > > > > > > I need an answer from you and Sam whether I can create new headers. > > > > > > For me, it is the right thing to do. > > > > Please follow the advice of Alex for the radeon driver. > > Great. Thanks for resolving this Sam. > > Will fix all occurrences. I'm not really following these patch sets you are sending out. They all seem completely independent. I was expecting updated versions with feedback integrated, but they seem to be just different fixes. Are you planning to send out updated versions based on feedback from these series? Also, some of the patches have subtle errors in them (e.g., wrong descriptions of variables, wrong copyright headers on new files, etc.), do you mind if I fix them up locally when applying or would you rather I point out the changes and you can integrate them and resend? Thanks, Alex > > -- > Lee Jones [李琼斯] > Senior Technical Lead - Developer Services > Linaro.org │ Open source software for Arm SoCs > Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH 15/20] drm/radeon/r600d: Move 'rc600_*' prototypes into shared header
On Tue, 10 Nov 2020, Sam Ravnborg wrote: > Hi Lee, > > > > the *d.h headers are supposed to just be hardware definitions. I'd > > > prefer to keep driver stuff out of them. > > > > That's fine (I did wonder if that were the case). > > > > I need an answer from you and Sam whether I can create new headers. > > > > For me, it is the right thing to do. > > Please follow the advice of Alex for the radeon driver. Great. Thanks for resolving this Sam. Will fix all occurrences. -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH 15/20] drm/radeon/r600d: Move 'rc600_*' prototypes into shared header
Hi Lee, > > the *d.h headers are supposed to just be hardware definitions. I'd > > prefer to keep driver stuff out of them. > > That's fine (I did wonder if that were the case). > > I need an answer from you and Sam whether I can create new headers. > > For me, it is the right thing to do. Please follow the advice of Alex for the radeon driver. Sam
Re: [PATCH 15/20] drm/radeon/r600d: Move 'rc600_*' prototypes into shared header
On Mon, 09 Nov 2020, Alex Deucher wrote: > On Mon, Nov 9, 2020 at 4:19 PM Lee Jones wrote: > > > > Fixes the following W=1 kernel build warning(s): > > > > drivers/gpu/drm/radeon/r600_hdmi.c:177:6: warning: no previous prototype > > for ‘r600_hdmi_update_acr’ [-Wmissing-prototypes] > > 177 | void r600_hdmi_update_acr(struct drm_encoder *encoder, long offset, > > | ^~~~ > > drivers/gpu/drm/radeon/r600_hdmi.c:217:6: warning: no previous prototype > > for ‘r600_set_avi_packet’ [-Wmissing-prototypes] > > 217 | void r600_set_avi_packet(struct radeon_device *rdev, u32 offset, > > | ^~~ > > drivers/gpu/drm/radeon/r600_hdmi.c:314:6: warning: no previous prototype > > for ‘r600_hdmi_audio_set_dto’ [-Wmissing-prototypes] > > 314 | void r600_hdmi_audio_set_dto(struct radeon_device *rdev, > > | ^~~ > > drivers/gpu/drm/radeon/r600_hdmi.c:340:6: warning: no previous prototype > > for ‘r600_set_vbi_packet’ [-Wmissing-prototypes] > > 340 | void r600_set_vbi_packet(struct drm_encoder *encoder, u32 offset) > > | ^~~ > > drivers/gpu/drm/radeon/r600_hdmi.c:351:6: warning: no previous prototype > > for ‘r600_set_audio_packet’ [-Wmissing-prototypes] > > 351 | void r600_set_audio_packet(struct drm_encoder *encoder, u32 offset) > > | ^ > > drivers/gpu/drm/radeon/r600_hdmi.c:393:6: warning: no previous prototype > > for ‘r600_set_mute’ [-Wmissing-prototypes] > > 393 | void r600_set_mute(struct drm_encoder *encoder, u32 offset, bool > > mute) > > | ^ > > drivers/gpu/drm/radeon/r600_hdmi.c:469:6: warning: no previous prototype > > for ‘r600_hdmi_enable’ [-Wmissing-prototypes] > > 469 | void r600_hdmi_enable(struct drm_encoder *encoder, bool enable) > > | ^~~~ > > > > Cc: Alex Deucher > > Cc: "Christian König" > > Cc: David Airlie > > Cc: Daniel Vetter > > Cc: amd-...@lists.freedesktop.org > > Cc: dri-de...@lists.freedesktop.org > > Signed-off-by: Lee Jones > > --- > > drivers/gpu/drm/radeon/r600d.h| 14 ++ > > drivers/gpu/drm/radeon/radeon_audio.c | 11 +-- > > 2 files changed, 15 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/radeon/r600d.h b/drivers/gpu/drm/radeon/r600d.h > > index 2e00a5287bd2d..db4bcc8bee4e5 100644 > > --- a/drivers/gpu/drm/radeon/r600d.h > > +++ b/drivers/gpu/drm/radeon/r600d.h > > @@ -27,6 +27,20 @@ > > #ifndef R600D_H > > #define R600D_H > > > > +struct radeon_crtc; > > +struct radeon_hdmi_acr; > > + > > +void r600_set_audio_packet(struct drm_encoder *encoder, u32 offset); > > +void r600_set_mute(struct drm_encoder *encoder, u32 offset, bool mute); > > +void r600_hdmi_audio_set_dto(struct radeon_device *rdev, > > + struct radeon_crtc *crtc, unsigned int clock); > > +void r600_set_avi_packet(struct radeon_device *rdev, u32 offset, > > + unsigned char *buffer, size_t size); > > +void r600_hdmi_update_acr(struct drm_encoder *encoder, long offset, > > + const struct radeon_hdmi_acr *acr); > > +void r600_set_vbi_packet(struct drm_encoder *encoder, u32 offset); > > +void r600_hdmi_enable(struct drm_encoder *encoder, bool enable); > > + > > the *d.h headers are supposed to just be hardware definitions. I'd > prefer to keep driver stuff out of them. That's fine (I did wonder if that were the case). I need an answer from you and Sam whether I can create new headers. For me, it is the right thing to do. -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH 15/20] drm/radeon/r600d: Move 'rc600_*' prototypes into shared header
On Mon, Nov 9, 2020 at 4:19 PM Lee Jones wrote: > > Fixes the following W=1 kernel build warning(s): > > drivers/gpu/drm/radeon/r600_hdmi.c:177:6: warning: no previous prototype for > ‘r600_hdmi_update_acr’ [-Wmissing-prototypes] > 177 | void r600_hdmi_update_acr(struct drm_encoder *encoder, long offset, > | ^~~~ > drivers/gpu/drm/radeon/r600_hdmi.c:217:6: warning: no previous prototype for > ‘r600_set_avi_packet’ [-Wmissing-prototypes] > 217 | void r600_set_avi_packet(struct radeon_device *rdev, u32 offset, > | ^~~ > drivers/gpu/drm/radeon/r600_hdmi.c:314:6: warning: no previous prototype for > ‘r600_hdmi_audio_set_dto’ [-Wmissing-prototypes] > 314 | void r600_hdmi_audio_set_dto(struct radeon_device *rdev, > | ^~~ > drivers/gpu/drm/radeon/r600_hdmi.c:340:6: warning: no previous prototype for > ‘r600_set_vbi_packet’ [-Wmissing-prototypes] > 340 | void r600_set_vbi_packet(struct drm_encoder *encoder, u32 offset) > | ^~~ > drivers/gpu/drm/radeon/r600_hdmi.c:351:6: warning: no previous prototype for > ‘r600_set_audio_packet’ [-Wmissing-prototypes] > 351 | void r600_set_audio_packet(struct drm_encoder *encoder, u32 offset) > | ^ > drivers/gpu/drm/radeon/r600_hdmi.c:393:6: warning: no previous prototype for > ‘r600_set_mute’ [-Wmissing-prototypes] > 393 | void r600_set_mute(struct drm_encoder *encoder, u32 offset, bool mute) > | ^ > drivers/gpu/drm/radeon/r600_hdmi.c:469:6: warning: no previous prototype for > ‘r600_hdmi_enable’ [-Wmissing-prototypes] > 469 | void r600_hdmi_enable(struct drm_encoder *encoder, bool enable) > | ^~~~ > > Cc: Alex Deucher > Cc: "Christian König" > Cc: David Airlie > Cc: Daniel Vetter > Cc: amd-...@lists.freedesktop.org > Cc: dri-de...@lists.freedesktop.org > Signed-off-by: Lee Jones > --- > drivers/gpu/drm/radeon/r600d.h| 14 ++ > drivers/gpu/drm/radeon/radeon_audio.c | 11 +-- > 2 files changed, 15 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/r600d.h b/drivers/gpu/drm/radeon/r600d.h > index 2e00a5287bd2d..db4bcc8bee4e5 100644 > --- a/drivers/gpu/drm/radeon/r600d.h > +++ b/drivers/gpu/drm/radeon/r600d.h > @@ -27,6 +27,20 @@ > #ifndef R600D_H > #define R600D_H > > +struct radeon_crtc; > +struct radeon_hdmi_acr; > + > +void r600_set_audio_packet(struct drm_encoder *encoder, u32 offset); > +void r600_set_mute(struct drm_encoder *encoder, u32 offset, bool mute); > +void r600_hdmi_audio_set_dto(struct radeon_device *rdev, > + struct radeon_crtc *crtc, unsigned int clock); > +void r600_set_avi_packet(struct radeon_device *rdev, u32 offset, > + unsigned char *buffer, size_t size); > +void r600_hdmi_update_acr(struct drm_encoder *encoder, long offset, > + const struct radeon_hdmi_acr *acr); > +void r600_set_vbi_packet(struct drm_encoder *encoder, u32 offset); > +void r600_hdmi_enable(struct drm_encoder *encoder, bool enable); > + the *d.h headers are supposed to just be hardware definitions. I'd prefer to keep driver stuff out of them. Alex > #define CP_PACKET2 0x8000 > #definePACKET2_PAD_SHIFT 0 > #definePACKET2_PAD_MASK(0x3fff << 0) > diff --git a/drivers/gpu/drm/radeon/radeon_audio.c > b/drivers/gpu/drm/radeon/radeon_audio.c > index 8c63ccb8b6235..a2be60aa3cec4 100644 > --- a/drivers/gpu/drm/radeon/radeon_audio.c > +++ b/drivers/gpu/drm/radeon/radeon_audio.c > @@ -27,6 +27,7 @@ > #include > #include "radeon.h" > #include "atom.h" > +#include "r600d.h" > #include "radeon_audio.h" > > void r600_audio_enable(struct radeon_device *rdev, struct r600_audio_pin > *pin, > @@ -63,8 +64,6 @@ void dce6_afmt_write_latency_fields(struct drm_encoder > *encoder, > struct r600_audio_pin* r600_audio_get_pin(struct radeon_device *rdev); > struct r600_audio_pin* dce6_audio_get_pin(struct radeon_device *rdev); > void dce6_afmt_select_pin(struct drm_encoder *encoder); > -void r600_hdmi_audio_set_dto(struct radeon_device *rdev, > - struct radeon_crtc *crtc, unsigned int clock); > void dce3_2_audio_set_dto(struct radeon_device *rdev, > struct radeon_crtc *crtc, unsigned int clock); > void dce4_hdmi_audio_set_dto(struct radeon_device *rdev, > @@ -75,31 +74,23 @@ void dce6_hdmi_audio_set_dto(struct radeon_device *rdev, > struct radeon_crtc *crtc, unsigned int clock); > void dce6_dp_audio_set_dto(struct radeon_device *rdev, > struct radeon_crtc *crtc, unsigned int clock); > -void r600_set_avi_packet(struct radeon_device *rdev, u32 offset, > - unsigned char *buffer, size_t size); > void evergreen_set_avi_packet(struct radeon_device *rdev, u32 offset, > unsigned char *buffer, size_t size); > -void r600_hdmi_update_acr(struct drm_encoder *encoder, long offset, > - const struct radeon_hdmi_acr *acr); > void dce3_2_hdmi_update_acr(struct
[PATCH 15/20] drm/radeon/r600d: Move 'rc600_*' prototypes into shared header
Fixes the following W=1 kernel build warning(s): drivers/gpu/drm/radeon/r600_hdmi.c:177:6: warning: no previous prototype for ‘r600_hdmi_update_acr’ [-Wmissing-prototypes] 177 | void r600_hdmi_update_acr(struct drm_encoder *encoder, long offset, | ^~~~ drivers/gpu/drm/radeon/r600_hdmi.c:217:6: warning: no previous prototype for ‘r600_set_avi_packet’ [-Wmissing-prototypes] 217 | void r600_set_avi_packet(struct radeon_device *rdev, u32 offset, | ^~~ drivers/gpu/drm/radeon/r600_hdmi.c:314:6: warning: no previous prototype for ‘r600_hdmi_audio_set_dto’ [-Wmissing-prototypes] 314 | void r600_hdmi_audio_set_dto(struct radeon_device *rdev, | ^~~ drivers/gpu/drm/radeon/r600_hdmi.c:340:6: warning: no previous prototype for ‘r600_set_vbi_packet’ [-Wmissing-prototypes] 340 | void r600_set_vbi_packet(struct drm_encoder *encoder, u32 offset) | ^~~ drivers/gpu/drm/radeon/r600_hdmi.c:351:6: warning: no previous prototype for ‘r600_set_audio_packet’ [-Wmissing-prototypes] 351 | void r600_set_audio_packet(struct drm_encoder *encoder, u32 offset) | ^ drivers/gpu/drm/radeon/r600_hdmi.c:393:6: warning: no previous prototype for ‘r600_set_mute’ [-Wmissing-prototypes] 393 | void r600_set_mute(struct drm_encoder *encoder, u32 offset, bool mute) | ^ drivers/gpu/drm/radeon/r600_hdmi.c:469:6: warning: no previous prototype for ‘r600_hdmi_enable’ [-Wmissing-prototypes] 469 | void r600_hdmi_enable(struct drm_encoder *encoder, bool enable) | ^~~~ Cc: Alex Deucher Cc: "Christian König" Cc: David Airlie Cc: Daniel Vetter Cc: amd-...@lists.freedesktop.org Cc: dri-de...@lists.freedesktop.org Signed-off-by: Lee Jones --- drivers/gpu/drm/radeon/r600d.h| 14 ++ drivers/gpu/drm/radeon/radeon_audio.c | 11 +-- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/radeon/r600d.h b/drivers/gpu/drm/radeon/r600d.h index 2e00a5287bd2d..db4bcc8bee4e5 100644 --- a/drivers/gpu/drm/radeon/r600d.h +++ b/drivers/gpu/drm/radeon/r600d.h @@ -27,6 +27,20 @@ #ifndef R600D_H #define R600D_H +struct radeon_crtc; +struct radeon_hdmi_acr; + +void r600_set_audio_packet(struct drm_encoder *encoder, u32 offset); +void r600_set_mute(struct drm_encoder *encoder, u32 offset, bool mute); +void r600_hdmi_audio_set_dto(struct radeon_device *rdev, + struct radeon_crtc *crtc, unsigned int clock); +void r600_set_avi_packet(struct radeon_device *rdev, u32 offset, + unsigned char *buffer, size_t size); +void r600_hdmi_update_acr(struct drm_encoder *encoder, long offset, + const struct radeon_hdmi_acr *acr); +void r600_set_vbi_packet(struct drm_encoder *encoder, u32 offset); +void r600_hdmi_enable(struct drm_encoder *encoder, bool enable); + #define CP_PACKET2 0x8000 #definePACKET2_PAD_SHIFT 0 #definePACKET2_PAD_MASK(0x3fff << 0) diff --git a/drivers/gpu/drm/radeon/radeon_audio.c b/drivers/gpu/drm/radeon/radeon_audio.c index 8c63ccb8b6235..a2be60aa3cec4 100644 --- a/drivers/gpu/drm/radeon/radeon_audio.c +++ b/drivers/gpu/drm/radeon/radeon_audio.c @@ -27,6 +27,7 @@ #include #include "radeon.h" #include "atom.h" +#include "r600d.h" #include "radeon_audio.h" void r600_audio_enable(struct radeon_device *rdev, struct r600_audio_pin *pin, @@ -63,8 +64,6 @@ void dce6_afmt_write_latency_fields(struct drm_encoder *encoder, struct r600_audio_pin* r600_audio_get_pin(struct radeon_device *rdev); struct r600_audio_pin* dce6_audio_get_pin(struct radeon_device *rdev); void dce6_afmt_select_pin(struct drm_encoder *encoder); -void r600_hdmi_audio_set_dto(struct radeon_device *rdev, - struct radeon_crtc *crtc, unsigned int clock); void dce3_2_audio_set_dto(struct radeon_device *rdev, struct radeon_crtc *crtc, unsigned int clock); void dce4_hdmi_audio_set_dto(struct radeon_device *rdev, @@ -75,31 +74,23 @@ void dce6_hdmi_audio_set_dto(struct radeon_device *rdev, struct radeon_crtc *crtc, unsigned int clock); void dce6_dp_audio_set_dto(struct radeon_device *rdev, struct radeon_crtc *crtc, unsigned int clock); -void r600_set_avi_packet(struct radeon_device *rdev, u32 offset, - unsigned char *buffer, size_t size); void evergreen_set_avi_packet(struct radeon_device *rdev, u32 offset, unsigned char *buffer, size_t size); -void r600_hdmi_update_acr(struct drm_encoder *encoder, long offset, - const struct radeon_hdmi_acr *acr); void dce3_2_hdmi_update_acr(struct drm_encoder *encoder, long offset, const struct radeon_hdmi_acr *acr); void evergreen_hdmi_update_acr(struct drm_encoder *encoder, long offset, const struct radeon_hdmi_acr *acr); -void r600_set_vbi_packet(struct drm_encoder *encoder, u32 offset); void dce4_set_vbi_packet(struct drm_encoder *encoder, u32 offset); void
