Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hdadma : DMA flow unification #114

Closed
wants to merge 9 commits into from

Conversation

awladzin
Copy link
Collaborator

It is a bit big package, but after consulting our DL Marcin, we decided that only in this way we will not crash CI.

Marcin Maka and others added 9 commits June 27, 2018 16:01
Prepared for hda stream_tag, same for host/dai and dw-dma/hda-dma.

Signed-off-by: Marcin Maka <[email protected]>
This commit enable hda-dma in FW, also improve sending callback function by
checking write pointer position in buffer.
Signed-off-by: Adam Wladzinski <[email protected]>
Signed-off-by: Adam Wladzinski <[email protected]>
Signed-off-by: Adam Wladzinski <[email protected]>
Signed-off-by: Adam Wladzinski <[email protected]>
Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I've only done a partial review since I can see some of the patches are needing to be squashed. Can you also add commit messages (why and how changes are being made) and do the PR against the next branch (sinec master is stabilising).

{
struct dai *dai = (struct dai *)data;
struct dma_pdata *p = dma_get_drvdata(dma);
int32_t gval;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indentation looks wrong.

*/
*/

uint32_t dgbwp = host_dma_reg_read(dma, channel, DGBWP);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All variable declerations should be at the top.

@lgirdwood
Copy link
Member

oh, and please also delete the merge commit. thanks.

@plbossart
Copy link
Member

plbossart commented Jul 16, 2018

Kind reminder that we try to use the Linux style, and I get tons of issues reported by checkpatch.pl.

CHECK: Comparison to NULL could be written "!config->elem_array.elems"
#53: FILE: src/audio/dai.c:269:
+	if (config->elem_array.elems == NULL) {

CHECK: Alignment should match open parenthesis
#55: FILE: src/audio/dai.c:271:
+		err = dma_sg_alloc(&config->elem_array,
+			config->direction,

CHECK: Comparison to NULL could be written "!config->elem_array.elems"
#108: FILE: src/audio/dai.c:317:
+	if (config->elem_array.elems == NULL) {

CHECK: Alignment should match open parenthesis
#110: FILE: src/audio/dai.c:319:
+		err = dma_sg_alloc(&config->elem_array,
+			config->direction,

CHECK: Comparison to NULL could be written "!dd->config.elem_array.elems"
#138: FILE: src/audio/dai.c:408:
+	if (dd->config.elem_array.elems == NULL) {

CHECK: Unnecessary parentheses around 'hc->current'
#218: FILE: src/audio/host.c:103:
+	if (++(hc->current) == hc->elem_array.count)

CHECK: if this code is redundant consider removing it
#289: FILE: src/audio/host.c:255:
+#if 0

CHECK: if this code is redundant consider removing it
#296: FILE: src/audio/host.c:262:
+#if 0

CHECK: if this code is redundant consider removing it
#338: FILE: src/audio/host.c:304:
+#if 0

CHECK: Lines should not end with a '('
#401: FILE: src/audio/host.c:380:
+	err = dma_sg_alloc(

CHECK: if this code is redundant consider removing it
#417: FILE: src/audio/host.c:396:
+#if 0

CHECK: Alignment should match open parenthesis
#918: FILE: src/include/sof/dma.h:276:
+int dma_sg_alloc(struct dma_sg_elem_array *elem_array, uint32_t direction,
+	uint32_t period_count, uint32_t period_bytes,

CHECK: Alignment should match open parenthesis
#970: FILE: src/ipc/dma-copy.c:91:
+int dma_copy_to_host(struct dma_copy *dc, struct dma_sg_config *host_sg,
+	int32_t host_offset, void *local_ptr, int32_t size)

CHECK: Alignment should match open parenthesis
#1003: FILE: src/ipc/dma-copy.c:256:
+int dma_copy_from_host(struct dma_copy *dc, struct dma_sg_config *host_sg,
+	int32_t host_offset, void *local_ptr, int32_t size)

CHECK: Alignment should match open parenthesis
#1142: FILE: src/lib/dma-trace.c:220:
+	err = dma_sg_alloc(&config.elem_array, config.direction,
+			elem_num, elem_size, elem_addr, 0);

CHECK: Alignment should match open parenthesis
#1166: FILE: src/lib/dma.c:93:
+int dma_sg_alloc(struct dma_sg_elem_array *elem_array, uint32_t direction,
+	uint32_t period_count, uint32_t period_bytes,

CHECK: Alignment should match open parenthesis
#1172: FILE: src/lib/dma.c:99:
+	elem_array->elems = rzalloc(RZONE_RUNTIME, SOF_MEM_CAPS_RAM,
+		sizeof(struct dma_sg_elem) * period_count);

CHECK: Comparison to NULL could be written "!elem_array->elems"
#1173: FILE: src/lib/dma.c:100:
+	if (elem_array->elems == NULL)

ERROR: Missing Signed-off-by: line(s)

CHECK: Please use a blank line after function/struct/union/enum declarations
#16: FILE: src/drivers/hda-dma.c:120:
 }
+static uint64_t hda_work(void *data, uint64_t delay)

CHECK: Blank lines aren't necessary after an open brace '{'
#36: FILE: src/drivers/hda-dma.c:139:
 {
+

CHECK: Please don't use multiple blank lines
#37: FILE: src/drivers/hda-dma.c:140:
+
+

ERROR: space required after that ',' (ctx:VxV)
#60: FILE: src/drivers/hda-dma.c:156:
+	work_schedule_default(dma,HDA_LINK_1MS_US);
 	                         ^

WARNING: suspect code indent for conditional statements (8, 12)
#63: FILE: src/drivers/hda-dma.c:159:
+	if ((dgbwp << 8) == 0 && (dgbrp << 8) != 0) {
+	    if (p->chan[channel].cb) {

WARNING: Statements should start on a tabstop
#64: FILE: src/drivers/hda-dma.c:160:
+	    if (p->chan[channel].cb) {

CHECK: Please don't use multiple blank lines
#74: FILE: src/drivers/hda-dma.c:167:
 
+

CHECK: Blank lines aren't necessary before a close brace '}'
#90: FILE: src/drivers/hda-dma.c:183:
 
+	}

CHECK: Alignment should match open parenthesis
#84: FILE: src/drivers/hda-dma.c:250:
+	work_schedule_default(hda_dma_copy(dma, channel,
+			p->chan[channel].cb_data),

@ufoq
Copy link

ufoq commented Jul 16, 2018

@plbossart I think you have to escape '#' char cos you are referencing a lot of random pull requestes

@plbossart
Copy link
Member

@mUfoq i fixed the comments, please fix your code...

Copy link
Contributor

@keyonjie keyonjie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

too big a commit. you should spit them for easy review, e.g. 1st: add dma_sg_alloc/free; 2nd: replace existed list with array. and you may need specify why you are doing this change.

@jiawang6
Copy link

CI observed building failure.

Failed Platform: baytrail apollolake cannonlake

configure: error: in `/build/sof/src':
configure: error: C compiler cannot create executables

@lgirdwood
Copy link
Member

@jiawang6 this PR will be is on a version of master that does not have configure.ac fix. It will be rebased soon and will be getting merged into -next (after squash/rebase).

@awladzin awladzin closed this Jul 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants